{"thread":{"id":"31043","subject":"possible bug in autocompletion","startedAt":"2012-07-17T09:10:39Z","lastAt":"2012-09-19T23:43:32Z","messageCount":10,"participants":["Jeroen Meijer","Jeff King","Felipe Contreras","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"195195","messageId":"BLU0-SMTP405CDB35308082B180185A6B4DB0@phx.gbl","threadId":"31043","inReplyTo":null,"subject":"possible bug in autocompletion","fromName":"Jeroen Meijer","fromEmail":"jjgmeijer@hotmail.com","sentAt":"2012-07-17T09:10:39Z","receivedAt":"2012-07-17T09:10:39Z","isPatch":false,"sender":{"key":"jjgmeijer@hotmail.com","avatar":null},"body":"We have a tag name with some special characters. The tag name is \n'Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721'. In \nsomecases where autocompletion is used an error is given, such as 'bash: \nBuild%20V%20${bamboo.custom.jiraversion.name}%20Build%20721: bad \nsubstitution'. This can be invoked by typing 'git checkout B' and then \npressing tab.\nOf course; the tag is useless but still I guess this is a bug in the \nautocompletion of git.\n\nRegards,\n\nMeijuh\n"},{"id":"195200","messageId":"20120717121232.GA32571@sigill.intra.peff.net","threadId":"31043","inReplyTo":"BLU0-SMTP405CDB35308082B180185A6B4DB0@phx.gbl","subject":"Re: possible bug in autocompletion","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-17T12:12:32Z","receivedAt":"2012-07-17T12:12:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 17, 2012 at 11:10:39AM +0200, Jeroen Meijer wrote:\n\n> We have a tag name with some special characters. The tag name is\n> 'Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721'. In\n> somecases where autocompletion is used an error is given, such as\n> 'bash: Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721:\n> bad substitution'. This can be invoked by typing 'git checkout B' and\n> then pressing tab.\n\nHrm. Weird. It is the \"${}\" in your tag name that causes the problem,\nand it all boils down to bash trying to do parameter expansion on the\ncontents of \"compgen -W\". You can see it in a much simpler example:\n\n  $ echo '${foo.bar}' ;# no expansion, works\n  ${foo.bar}\n\n  $ echo \"${foo.bar}\" ;# expansion, bash rightfully complains\n  bash: ${foo.bar}: bad substitution\n\n  $ compgen -W '${foo.bar}' f\n  bash: ${foo.bar}: bad substitution\n\nIn the final command, we use single-quotes so there is no expansion\nbefore the command execution. So it happens internally to compgen.\ndocumentation for compgen says:\n\n  -W wordlist\n          The  wordlist is split using the characters in the IFS special vari‐\n          able as delimiters, and each resultant word is expanded.  The possi‐\n          ble  completions  are  the members of the resultant list which match\n          the word being completed.\n\nWhich seems kind of crazy to me. It means that we need to be quoting\neverything we feed to compgen to avoid accidental expansion. But I guess\nbash is not likely to change anytime soon, so we probably need to work\naround it.\n\n> Of course; the tag is useless but still I guess this is a bug in the\n> autocompletion of git.\n\nYeah, that tag is crazy. But this can happen anywhere that we feed\narbitrary data to compgen. Try this:\n\n  echo content >'${foo.bar}' &&\n  git add . &&\n  git commit\n\n  git show HEAD:<tab>\n\nwhich generates the same error. Or even a file named \"foo$bar\", which is\nmuch more likely; that will not generate an error, but it will expand\n$bar and produce erroneous results. I think we also have issues with\nfiles with single and double quotes in them.\n\nSomething like this seems to fix it for me:\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex ffedce7..2d20824 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -261,7 +261,12 @@ __gitcomp ()\n __gitcomp_nl ()\n {\n \tlocal IFS=$'\\n'\n-\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n+\tlocal words=$1\n+\twords=${words//\\\\/\\\\\\\\}\n+\twords=${words//\\$/\\\\\\$}\n+\twords=${words//\\'/\\\\\\'}\n+\twords=${words//\\\"/\\\\\\\"}\n+\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n }\n \n __git_heads ()\n\nbut it is awfully ugly. Maybe completion experts can offer a better\nsolution.\n\n-Peff\n"},{"id":"199483","messageId":"CAMP44s2X5-BUyLtkTqGMa6w5K6uT25YLEp+Q2TdVR_qCObOpeA@mail.gmail.com","threadId":"31043","inReplyTo":"20120717121232.GA32571@sigill.intra.peff.net","subject":"Re: possible bug in autocompletion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-09-19T17:08:09Z","receivedAt":"2012-09-19T17:08:09Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote:\n\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -261,7 +261,12 @@ __gitcomp ()\n>  __gitcomp_nl ()\n>  {\n>         local IFS=$'\\n'\n> -       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n> +       local words=$1\n> +       words=${words//\\\\/\\\\\\\\}\n> +       words=${words//\\$/\\\\\\$}\n> +       words=${words//\\'/\\\\\\'}\n> +       words=${words//\\\"/\\\\\\\"}\n> +       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n>  }\n\nWhat about something like this?\n\nlocal words\nprintf -v words \"%q\" \"$w\"\nCOMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"199495","messageId":"20120919174336.GA11699@sigill.intra.peff.net","threadId":"31043","inReplyTo":"CAMP44s2X5-BUyLtkTqGMa6w5K6uT25YLEp+Q2TdVR_qCObOpeA@mail.gmail.com","subject":"Re: possible bug in autocompletion","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-19T17:43:36Z","receivedAt":"2012-09-19T17:43:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 19, 2012 at 07:08:09PM +0200, Felipe Contreras wrote:\n\n> On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote:\n> \n> > --- a/contrib/completion/git-completion.bash\n> > +++ b/contrib/completion/git-completion.bash\n> > @@ -261,7 +261,12 @@ __gitcomp ()\n> >  __gitcomp_nl ()\n> >  {\n> >         local IFS=$'\\n'\n> > -       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n> > +       local words=$1\n> > +       words=${words//\\\\/\\\\\\\\}\n> > +       words=${words//\\$/\\\\\\$}\n> > +       words=${words//\\'/\\\\\\'}\n> > +       words=${words//\\\"/\\\\\\\"}\n> > +       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n> >  }\n> \n> What about something like this?\n> \n> local words\n> printf -v words \"%q\" \"$w\"\n> COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n\nThanks, I didn't know about bash's internal printf magic. That is a much\nmore elegant solution.\n\nCare to wrap it up in a patch?\n\n-Peff\n"},{"id":"199500","messageId":"7vipb9hp38.fsf@alter.siamese.dyndns.org","threadId":"31043","inReplyTo":"CAMP44s2X5-BUyLtkTqGMa6w5K6uT25YLEp+Q2TdVR_qCObOpeA@mail.gmail.com","subject":"Re: possible bug in autocompletion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-19T17:58:19Z","receivedAt":"2012-09-19T17:58:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote:\n>\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -261,7 +261,12 @@ __gitcomp ()\n>>  __gitcomp_nl ()\n>>  {\n>>         local IFS=$'\\n'\n>> -       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n>> +       local words=$1\n>> +       words=${words//\\\\/\\\\\\\\}\n>> +       words=${words//\\$/\\\\\\$}\n>> +       words=${words//\\'/\\\\\\'}\n>> +       words=${words//\\\"/\\\\\\\"}\n>> +       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n>>  }\n>\n> What about something like this?\n>\n> local words\n> printf -v words \"%q\" \"$w\"\n> COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n\nBoth \"printf -v\" and \"%q\" are brilliant ;-)\n"},{"id":"199503","messageId":"CAMP44s1Xvvs7g1quUEs2b43VciK2=Nt-AntJcwG0CPf6p8Xk_A@mail.gmail.com","threadId":"31043","inReplyTo":"20120919174336.GA11699@sigill.intra.peff.net","subject":"Re: possible bug in autocompletion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-09-19T18:16:22Z","receivedAt":"2012-09-19T18:16:22Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Sep 19, 2012 at 7:43 PM, Jeff King <peff@peff.net> wrote:\n> On Wed, Sep 19, 2012 at 07:08:09PM +0200, Felipe Contreras wrote:\n>\n>> On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote:\n>>\n>> > --- a/contrib/completion/git-completion.bash\n>> > +++ b/contrib/completion/git-completion.bash\n>> > @@ -261,7 +261,12 @@ __gitcomp ()\n>> >  __gitcomp_nl ()\n>> >  {\n>> >         local IFS=$'\\n'\n>> > -       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n>> > +       local words=$1\n>> > +       words=${words//\\\\/\\\\\\\\}\n>> > +       words=${words//\\$/\\\\\\$}\n>> > +       words=${words//\\'/\\\\\\'}\n>> > +       words=${words//\\\"/\\\\\\\"}\n>> > +       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n>> >  }\n>>\n>> What about something like this?\n>>\n>> local words\n>> printf -v words \"%q\" \"$w\"\n>> COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n>\n> Thanks, I didn't know about bash's internal printf magic. That is a much\n> more elegant solution.\n>\n> Care to wrap it up in a patch?\n\nI'm trying to, but unfortunately \"\\n\" gets converted to \"\\\\n\", so it\ndoesn't get separated to words. Any ideas?\n\n-- \nFelipe Contreras\n"},{"id":"199506","messageId":"CAMP44s1ZVTgBTQDyBHKvos-uSo0FeOO437MvTYH0YE0Lx-xDOA@mail.gmail.com","threadId":"31043","inReplyTo":"CAMP44s1Xvvs7g1quUEs2b43VciK2=Nt-AntJcwG0CPf6p8Xk_A@mail.gmail.com","subject":"Re: possible bug in autocompletion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-09-19T18:23:01Z","receivedAt":"2012-09-19T18:23:01Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Sep 19, 2012 at 8:16 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> On Wed, Sep 19, 2012 at 7:43 PM, Jeff King <peff@peff.net> wrote:\n>> On Wed, Sep 19, 2012 at 07:08:09PM +0200, Felipe Contreras wrote:\n>>\n>>> On Tue, Jul 17, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote:\n>>>\n>>> > --- a/contrib/completion/git-completion.bash\n>>> > +++ b/contrib/completion/git-completion.bash\n>>> > @@ -261,7 +261,12 @@ __gitcomp ()\n>>> >  __gitcomp_nl ()\n>>> >  {\n>>> >         local IFS=$'\\n'\n>>> > -       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n>>> > +       local words=$1\n>>> > +       words=${words//\\\\/\\\\\\\\}\n>>> > +       words=${words//\\$/\\\\\\$}\n>>> > +       words=${words//\\'/\\\\\\'}\n>>> > +       words=${words//\\\"/\\\\\\\"}\n>>> > +       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n>>> >  }\n>>>\n>>> What about something like this?\n>>>\n>>> local words\n>>> printf -v words \"%q\" \"$w\"\n>>> COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$words\" -- \"${3-$cur}\"))\n>>\n>> Thanks, I didn't know about bash's internal printf magic. That is a much\n>> more elegant solution.\n>>\n>> Care to wrap it up in a patch?\n>\n> I'm trying to, but unfortunately \"\\n\" gets converted to \"\\\\n\", so it\n> doesn't get separated to words. Any ideas?\n\nActually, this seems to do the trick:\n\n\tlocal words IFS=$'\\n'\n\tprintf -v words \"%q\" \"$1\"\n\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"${words//\\\\n/$IFS}\"\n-- \"${3-$cur}\"))\n\nI don't know how to do $'\\n' in the middle of double-quotes, but $IFS works.\n\n-- \nFelipe Contreras\n"},{"id":"199522","messageId":"20120919195518.GA22310@sigill.intra.peff.net","threadId":"31043","inReplyTo":"CAMP44s1ZVTgBTQDyBHKvos-uSo0FeOO437MvTYH0YE0Lx-xDOA@mail.gmail.com","subject":"Re: possible bug in autocompletion","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-19T19:55:18Z","receivedAt":"2012-09-19T19:55:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 19, 2012 at 08:23:01PM +0200, Felipe Contreras wrote:\n\n> >> Care to wrap it up in a patch?\n> >\n> > I'm trying to, but unfortunately \"\\n\" gets converted to \"\\\\n\", so it\n> > doesn't get separated to words. Any ideas?\n> \n> Actually, this seems to do the trick:\n> \n> \tlocal words IFS=$'\\n'\n> \tprintf -v words \"%q\" \"$1\"\n> \tCOMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"${words//\\\\n/$IFS}\"\n> -- \"${3-$cur}\"))\n> \n> I don't know how to do $'\\n' in the middle of double-quotes, but $IFS works.\n\nI don't think you can search-and-replace backslash-escaped characters\nand always get the correct result.  For example, in this string:\n\n  \\\\n\n\nYour regex would match the \"\\n\", missing the context that the backslash\nis actually the second-half of an escaped pair. Searching for \"[^\\\\]\\\\n\"\nwould similarly miss that:\n\n  \\\\\\n\n\nshould be converted. IOW, I think backslash-escaping fundamentally has\nto either be parsed left-to-right, or quoted backslashes peeled in order\n(if I were not such a bad computer scientist, I could probably come up\nwith some proof involving Chomsky's hierarchy of grammars).\n\nI think the real problem is that we are feeding \"printf %q\" an input\nwhere we want _most_ of the constructs quoted, but not some (namely\nnewline, which is syntactically significant, and which we accept will\nbreak our completion). So something like the manual quoting I posted\nearlier in the thread is actually much closer to what we want.\n\nOn a related note, I notice that even with either of our patches, doing\nthis:\n\n  echo content >'${foo.bar}'\n  git add .\n  git commit -m whatever\n  git show HEAD:<tab>\n\nwill yield this completion:\n\n  git show HEAD:${foo.bar}\n\nwhich is correct, but not useful. It needs to be quoted _again_ to avoid\ninterpolation when you actually quote the command. Bash's filename\ncompletion handles this automatically. E.g., if you do:\n\n  git add <tab>\n\nyou will get:\n\n  git add \\$\\{foo.bar\\}\n\nI have no idea if that internal to bash's filename completion, or if\nthere is some easy facility offered to programmable completions to do\nthe same thing.  I don't think this is a high priority, but it would be\nnice to handle it. And moreover, I am really wondering if we are missing\nsome solution that bash is providing to help us with the quoting issues.\nSurely we are not the first completion script to come up against this.\n\n-Peff\n"},{"id":"199543","messageId":"CAMP44s33b7uNg6G3m3wHEACa_wYwf_5==h64CHw2b86-rJd5VQ@mail.gmail.com","threadId":"31043","inReplyTo":"20120919195518.GA22310@sigill.intra.peff.net","subject":"Re: possible bug in autocompletion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-09-19T23:08:29Z","receivedAt":"2012-09-19T23:08:29Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Sep 19, 2012 at 9:55 PM, Jeff King <peff@peff.net> wrote:\n\n> I have no idea if that internal to bash's filename completion, or if\n> there is some easy facility offered to programmable completions to do\n> the same thing.  I don't think this is a high priority, but it would be\n> nice to handle it. And moreover, I am really wondering if we are missing\n> some solution that bash is providing to help us with the quoting issues.\n> Surely we are not the first completion script to come up against this.\n\nI found a much easier solution:\n\n-       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n+       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$(quote \"$1\")\"\n-- \"${3-$cur}\"))\n\nBut what about the people that don't have bash-completion?\n\nBTW:\n\nquote()\n{\n    local quoted=${1//\\'/\\'\\\\\\'\\'}\n    printf \"'%s'\" \"$quoted\"\n}\n\n-- \nFelipe Contreras\n"},{"id":"199550","messageId":"20120919234332.GB27626@sigill.intra.peff.net","threadId":"31043","inReplyTo":"CAMP44s33b7uNg6G3m3wHEACa_wYwf_5==h64CHw2b86-rJd5VQ@mail.gmail.com","subject":"Re: possible bug in autocompletion","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-19T23:43:32Z","receivedAt":"2012-09-19T23:43:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 20, 2012 at 01:08:29AM +0200, Felipe Contreras wrote:\n\n> On Wed, Sep 19, 2012 at 9:55 PM, Jeff King <peff@peff.net> wrote:\n> \n> > I have no idea if that internal to bash's filename completion, or if\n> > there is some easy facility offered to programmable completions to do\n> > the same thing.  I don't think this is a high priority, but it would be\n> > nice to handle it. And moreover, I am really wondering if we are missing\n> > some solution that bash is providing to help us with the quoting issues.\n> > Surely we are not the first completion script to come up against this.\n> \n> I found a much easier solution:\n> \n> -       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n> +       COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$(quote \"$1\")\"\n> -- \"${3-$cur}\"))\n\nOh, nice. :)\n\n> But what about the people that don't have bash-completion?\n> \n> BTW:\n> \n> quote()\n> {\n>     local quoted=${1//\\'/\\'\\\\\\'\\'}\n>     printf \"'%s'\" \"$quoted\"\n> }\n\nThat is short and obvious enough that we could probably just\ncut-and-paste it into our script as _git_quote (and it is basically a\ncleaner version of the thing that I posted).\n\n-Peff\n"}]}