* possible bug in autocompletion
@ 2012-07-17 9:10 Jeroen Meijer
2012-07-17 12:12 ` Jeff King
0 siblings, 1 reply; 10+ messages in thread
From: Jeroen Meijer @ 2012-07-17 9:10 UTC (permalink / raw)
To: git
We have a tag name with some special characters. The tag name is
'Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721'. In
somecases where autocompletion is used an error is given, such as 'bash:
Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721: bad
substitution'. This can be invoked by typing 'git checkout B' and then
pressing tab.
Of course; the tag is useless but still I guess this is a bug in the
autocompletion of git.
Regards,
Meijuh
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: possible bug in autocompletion 2012-07-17 9:10 possible bug in autocompletion Jeroen Meijer @ 2012-07-17 12:12 ` Jeff King 2012-09-19 17:08 ` Felipe Contreras 0 siblings, 1 reply; 10+ messages in thread From: Jeff King @ 2012-07-17 12:12 UTC (permalink / raw) To: Jeroen Meijer; +Cc: git On Tue, Jul 17, 2012 at 11:10:39AM +0200, Jeroen Meijer wrote: > We have a tag name with some special characters. The tag name is > 'Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721'. In > somecases where autocompletion is used an error is given, such as > 'bash: Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721: > bad substitution'. This can be invoked by typing 'git checkout B' and > then pressing tab. Hrm. Weird. It is the "${}" in your tag name that causes the problem, and it all boils down to bash trying to do parameter expansion on the contents of "compgen -W". You can see it in a much simpler example: $ echo '${foo.bar}' ;# no expansion, works ${foo.bar} $ echo "${foo.bar}" ;# expansion, bash rightfully complains bash: ${foo.bar}: bad substitution $ compgen -W '${foo.bar}' f bash: ${foo.bar}: bad substitution In the final command, we use single-quotes so there is no expansion before the command execution. So it happens internally to compgen. documentation for compgen says: -W wordlist The wordlist is split using the characters in the IFS special vari‐ able as delimiters, and each resultant word is expanded. The possi‐ ble completions are the members of the resultant list which match the word being completed. Which seems kind of crazy to me. It means that we need to be quoting everything we feed to compgen to avoid accidental expansion. But I guess bash is not likely to change anytime soon, so we probably need to work around it. > Of course; the tag is useless but still I guess this is a bug in the > autocompletion of git. Yeah, that tag is crazy. But this can happen anywhere that we feed arbitrary data to compgen. Try this: echo content >'${foo.bar}' && git add . && git commit git show HEAD:<tab> which generates the same error. Or even a file named "foo$bar", which is much more likely; that will not generate an error, but it will expand $bar and produce erroneous results. I think we also have issues with files with single and double quotes in them. Something like this seems to fix it for me: diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index ffedce7..2d20824 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -261,7 +261,12 @@ __gitcomp () __gitcomp_nl () { local IFS=$'\n' - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}")) + local words=$1 + words=${words//\\/\\\\} + words=${words//\$/\\\$} + words=${words//\'/\\\'} + words=${words//\"/\\\"} + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) } __git_heads () but it is awfully ugly. Maybe completion experts can offer a better solution. -Peff ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: possible bug in autocompletion 2012-07-17 12:12 ` Jeff King @ 2012-09-19 17:08 ` Felipe Contreras 2012-09-19 17:43 ` Jeff King 2012-09-19 17:58 ` Junio C Hamano 0 siblings, 2 replies; 10+ messages in thread From: Felipe Contreras @ 2012-09-19 17:08 UTC (permalink / raw) To: Jeff King; +Cc: Jeroen Meijer, git On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote: > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -261,7 +261,12 @@ __gitcomp () > __gitcomp_nl () > { > local IFS=$'\n' > - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}")) > + local words=$1 > + words=${words//\\/\\\\} > + words=${words//\$/\\\$} > + words=${words//\'/\\\'} > + words=${words//\"/\\\"} > + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) > } What about something like this? local words printf -v words "%q" "$w" COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) Cheers. -- Felipe Contreras ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: possible bug in autocompletion 2012-09-19 17:08 ` Felipe Contreras @ 2012-09-19 17:43 ` Jeff King 2012-09-19 18:16 ` Felipe Contreras 2012-09-19 17:58 ` Junio C Hamano 1 sibling, 1 reply; 10+ messages in thread From: Jeff King @ 2012-09-19 17:43 UTC (permalink / raw) To: Felipe Contreras; +Cc: Jeroen Meijer, git On Wed, Sep 19, 2012 at 07:08:09PM +0200, Felipe Contreras wrote: > On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote: > > > --- a/contrib/completion/git-completion.bash > > +++ b/contrib/completion/git-completion.bash > > @@ -261,7 +261,12 @@ __gitcomp () > > __gitcomp_nl () > > { > > local IFS=$'\n' > > - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}")) > > + local words=$1 > > + words=${words//\\/\\\\} > > + words=${words//\$/\\\$} > > + words=${words//\'/\\\'} > > + words=${words//\"/\\\"} > > + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) > > } > > What about something like this? > > local words > printf -v words "%q" "$w" > COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) Thanks, I didn't know about bash's internal printf magic. That is a much more elegant solution. Care to wrap it up in a patch? -Peff ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: possible bug in autocompletion 2012-09-19 17:43 ` Jeff King @ 2012-09-19 18:16 ` Felipe Contreras 2012-09-19 18:23 ` Felipe Contreras 0 siblings, 1 reply; 10+ messages in thread From: Felipe Contreras @ 2012-09-19 18:16 UTC (permalink / raw) To: Jeff King; +Cc: Jeroen Meijer, git On Wed, Sep 19, 2012 at 7:43 PM, Jeff King <peff@peff.net> wrote: > On Wed, Sep 19, 2012 at 07:08:09PM +0200, Felipe Contreras wrote: > >> On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote: >> >> > --- a/contrib/completion/git-completion.bash >> > +++ b/contrib/completion/git-completion.bash >> > @@ -261,7 +261,12 @@ __gitcomp () >> > __gitcomp_nl () >> > { >> > local IFS=$'\n' >> > - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}")) >> > + local words=$1 >> > + words=${words//\\/\\\\} >> > + words=${words//\$/\\\$} >> > + words=${words//\'/\\\'} >> > + words=${words//\"/\\\"} >> > + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) >> > } >> >> What about something like this? >> >> local words >> printf -v words "%q" "$w" >> COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) > > Thanks, I didn't know about bash's internal printf magic. That is a much > more elegant solution. > > Care to wrap it up in a patch? I'm trying to, but unfortunately "\n" gets converted to "\\n", so it doesn't get separated to words. Any ideas? -- Felipe Contreras ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: possible bug in autocompletion 2012-09-19 18:16 ` Felipe Contreras @ 2012-09-19 18:23 ` Felipe Contreras 2012-09-19 19:55 ` Jeff King 0 siblings, 1 reply; 10+ messages in thread From: Felipe Contreras @ 2012-09-19 18:23 UTC (permalink / raw) To: Jeff King; +Cc: Jeroen Meijer, git On Wed, Sep 19, 2012 at 8:16 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote: > On Wed, Sep 19, 2012 at 7:43 PM, Jeff King <peff@peff.net> wrote: >> On Wed, Sep 19, 2012 at 07:08:09PM +0200, Felipe Contreras wrote: >> >>> On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote: >>> >>> > --- a/contrib/completion/git-completion.bash >>> > +++ b/contrib/completion/git-completion.bash >>> > @@ -261,7 +261,12 @@ __gitcomp () >>> > __gitcomp_nl () >>> > { >>> > local IFS=$'\n' >>> > - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}")) >>> > + local words=$1 >>> > + words=${words//\\/\\\\} >>> > + words=${words//\$/\\\$} >>> > + words=${words//\'/\\\'} >>> > + words=${words//\"/\\\"} >>> > + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) >>> > } >>> >>> What about something like this? >>> >>> local words >>> printf -v words "%q" "$w" >>> COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) >> >> Thanks, I didn't know about bash's internal printf magic. That is a much >> more elegant solution. >> >> Care to wrap it up in a patch? > > I'm trying to, but unfortunately "\n" gets converted to "\\n", so it > doesn't get separated to words. Any ideas? Actually, this seems to do the trick: local words IFS=$'\n' printf -v words "%q" "$1" COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "${words//\\n/$IFS}" -- "${3-$cur}")) I don't know how to do $'\n' in the middle of double-quotes, but $IFS works. -- Felipe Contreras ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: possible bug in autocompletion 2012-09-19 18:23 ` Felipe Contreras @ 2012-09-19 19:55 ` Jeff King 2012-09-19 23:08 ` Felipe Contreras 0 siblings, 1 reply; 10+ messages in thread From: Jeff King @ 2012-09-19 19:55 UTC (permalink / raw) To: Felipe Contreras; +Cc: Jeroen Meijer, git On Wed, Sep 19, 2012 at 08:23:01PM +0200, Felipe Contreras wrote: > >> Care to wrap it up in a patch? > > > > I'm trying to, but unfortunately "\n" gets converted to "\\n", so it > > doesn't get separated to words. Any ideas? > > Actually, this seems to do the trick: > > local words IFS=$'\n' > printf -v words "%q" "$1" > COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "${words//\\n/$IFS}" > -- "${3-$cur}")) > > I don't know how to do $'\n' in the middle of double-quotes, but $IFS works. I don't think you can search-and-replace backslash-escaped characters and always get the correct result. For example, in this string: \\n Your regex would match the "\n", missing the context that the backslash is actually the second-half of an escaped pair. Searching for "[^\\]\\n" would similarly miss that: \\\n should be converted. IOW, I think backslash-escaping fundamentally has to either be parsed left-to-right, or quoted backslashes peeled in order (if I were not such a bad computer scientist, I could probably come up with some proof involving Chomsky's hierarchy of grammars). I think the real problem is that we are feeding "printf %q" an input where we want _most_ of the constructs quoted, but not some (namely newline, which is syntactically significant, and which we accept will break our completion). So something like the manual quoting I posted earlier in the thread is actually much closer to what we want. On a related note, I notice that even with either of our patches, doing this: echo content >'${foo.bar}' git add . git commit -m whatever git show HEAD:<tab> will yield this completion: git show HEAD:${foo.bar} which is correct, but not useful. It needs to be quoted _again_ to avoid interpolation when you actually quote the command. Bash's filename completion handles this automatically. E.g., if you do: git add <tab> you will get: git add \$\{foo.bar\} I have no idea if that internal to bash's filename completion, or if there is some easy facility offered to programmable completions to do the same thing. I don't think this is a high priority, but it would be nice to handle it. And moreover, I am really wondering if we are missing some solution that bash is providing to help us with the quoting issues. Surely we are not the first completion script to come up against this. -Peff ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: possible bug in autocompletion 2012-09-19 19:55 ` Jeff King @ 2012-09-19 23:08 ` Felipe Contreras 2012-09-19 23:43 ` Jeff King 0 siblings, 1 reply; 10+ messages in thread From: Felipe Contreras @ 2012-09-19 23:08 UTC (permalink / raw) To: Jeff King; +Cc: Jeroen Meijer, git On Wed, Sep 19, 2012 at 9:55 PM, Jeff King <peff@peff.net> wrote: > I have no idea if that internal to bash's filename completion, or if > there is some easy facility offered to programmable completions to do > the same thing. I don't think this is a high priority, but it would be > nice to handle it. And moreover, I am really wondering if we are missing > some solution that bash is providing to help us with the quoting issues. > Surely we are not the first completion script to come up against this. I found a much easier solution: - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}")) + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$(quote "$1")" -- "${3-$cur}")) But what about the people that don't have bash-completion? BTW: quote() { local quoted=${1//\'/\'\\\'\'} printf "'%s'" "$quoted" } -- Felipe Contreras ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: possible bug in autocompletion 2012-09-19 23:08 ` Felipe Contreras @ 2012-09-19 23:43 ` Jeff King 0 siblings, 0 replies; 10+ messages in thread From: Jeff King @ 2012-09-19 23:43 UTC (permalink / raw) To: Felipe Contreras; +Cc: Jeroen Meijer, git On Thu, Sep 20, 2012 at 01:08:29AM +0200, Felipe Contreras wrote: > On Wed, Sep 19, 2012 at 9:55 PM, Jeff King <peff@peff.net> wrote: > > > I have no idea if that internal to bash's filename completion, or if > > there is some easy facility offered to programmable completions to do > > the same thing. I don't think this is a high priority, but it would be > > nice to handle it. And moreover, I am really wondering if we are missing > > some solution that bash is providing to help us with the quoting issues. > > Surely we are not the first completion script to come up against this. > > I found a much easier solution: > > - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}")) > + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$(quote "$1")" > -- "${3-$cur}")) Oh, nice. :) > But what about the people that don't have bash-completion? > > BTW: > > quote() > { > local quoted=${1//\'/\'\\\'\'} > printf "'%s'" "$quoted" > } That is short and obvious enough that we could probably just cut-and-paste it into our script as _git_quote (and it is basically a cleaner version of the thing that I posted). -Peff ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: possible bug in autocompletion 2012-09-19 17:08 ` Felipe Contreras 2012-09-19 17:43 ` Jeff King @ 2012-09-19 17:58 ` Junio C Hamano 1 sibling, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2012-09-19 17:58 UTC (permalink / raw) To: Felipe Contreras; +Cc: Jeff King, Jeroen Meijer, git Felipe Contreras <felipe.contreras@gmail.com> writes: > On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote: > >> --- a/contrib/completion/git-completion.bash >> +++ b/contrib/completion/git-completion.bash >> @@ -261,7 +261,12 @@ __gitcomp () >> __gitcomp_nl () >> { >> local IFS=$'\n' >> - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}")) >> + local words=$1 >> + words=${words//\\/\\\\} >> + words=${words//\$/\\\$} >> + words=${words//\'/\\\'} >> + words=${words//\"/\\\"} >> + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) >> } > > What about something like this? > > local words > printf -v words "%q" "$w" > COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$words" -- "${3-$cur}")) Both "printf -v" and "%q" are brilliant ;-) ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-09-19 23:43 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-17 9:10 possible bug in autocompletion Jeroen Meijer 2012-07-17 12:12 ` Jeff King 2012-09-19 17:08 ` Felipe Contreras 2012-09-19 17:43 ` Jeff King 2012-09-19 18:16 ` Felipe Contreras 2012-09-19 18:23 ` Felipe Contreras 2012-09-19 19:55 ` Jeff King 2012-09-19 23:08 ` Felipe Contreras 2012-09-19 23:43 ` Jeff King 2012-09-19 17:58 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).