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
Jeff KingRe: possible bug in autocompletion
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 substitutionIn 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 commitgit 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
Felipe ContrerasRe: possible bug in autocompletion
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
Jeff KingRe: possible bug in autocompletion
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
Felipe ContrerasRe: possible bug in autocompletion
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
Felipe ContrerasRe: possible bug in autocompletion
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
Jeff KingRe: possible bug in autocompletion
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
Felipe ContrerasRe: possible bug in autocompletion
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
Jeff KingRe: possible bug in autocompletion
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
Junio C HamanoRe: possible bug in autocompletion
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 ;-)