Re: possible bug in autocompletion

2012-09-19 Thread Felipe Contreras
On Tue, Jul 17, 2012 at 2:12 PM, Jeff King p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: possible bug in autocompletion

2012-09-19 Thread Jeff King
On Wed, Sep 19, 2012 at 07:08:09PM +0200, Felipe Contreras wrote:

 On Tue, Jul 17, 2012 at 2:12 PM, Jeff King p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: possible bug in autocompletion

2012-09-19 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 On Tue, Jul 17, 2012 at 2:12 PM, Jeff King p...@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 ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: possible bug in autocompletion

2012-09-19 Thread Felipe Contreras
On Wed, Sep 19, 2012 at 7:43 PM, Jeff King p...@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 p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: possible bug in autocompletion

2012-07-17 Thread Jeff King
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html