git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] completion: improve shell expansion of items

From
Jeff King <peff@peff.net>
Date
Sep 27, 2012, 19:48 UTC
Message-ID
<20120927194856.GA10673@sigill.intra.peff.net>
In-Reply-To
<20120927064338.GA4789@sigill.intra.peff.net>
On Thu, Sep 27, 2012 at 02:43:38AM -0400, Jeff King wrote:
Show 20 quoted lines
> Ah. The problem is that most of the load comes from my patch 4/3, which
> does a separate iteration. Here are the numbers after just patch 3:
> 
>   $ time __gitcomp_nl "$refs"
>   real    0m0.344s
>   user    0m0.392s
>   sys     0m0.040s
> 
> Slower, but not nearly as painful. Here are the numbers using sed[1]
> instead:
> 
>   $ time __gitcomp_nl "$refs"
>   real    0m0.100s
>   user    0m0.084s
>   sys     0m0.016s
> 
> So a little slower, but probably acceptable. We could maybe do the same
> trick on the output side (although it is a little trickier there,
> because we need it in a bash array). Of course, this is Linux; the fork
> for sed is way more expensive on some systems.

So something like the patch below does the quoting correctly (try committing a file like "name with spaces" and doing "git show HEAD:<TAB>"), and isn't too much slower:

  real    0m0.114s
  user    0m0.108s
  sys     0m0.004s

That's almost double the time without handling quoting, but keep in mind this is on 10,000 entries (and the real sluggishness we are trying to avoid is an order of magnitude). So it might be acceptable.

This is just a proof-of-concept patch. We'd probably want to replace the perl below with a more complicated sed invocation for portability (the trickiness is that the output is shown to the user, so we very much don't want to quote anything that does not have to be).

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index be800e0..20c09ef 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,6 +225,17 @@ fi
 fi
 fi
 
+# Quotes each element of a newline-delimited list for shell reuse
+__git_quote()
+{
+	echo "$1" |
+	sed "
+	  s/'/'\\\\''/g
+	  s/^/'/
+	  s/$/'/
+	"
+}
+
 # Generates completion reply with compgen, appending a space to possible
 # completion words, if necessary.
 # It accepts 1 to 4 arguments:
@@ -261,7 +272,10 @@ __gitcomp_nl ()
 __gitcomp_nl ()
 {
 	local IFS=$'\n'
-	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
+	COMPREPLY=($(
+	  compgen -P "${2-}" -S "${4- }" -W "$(__git_quote "$1")" -- "${3-$cur}" |
+	  perl -lne '/(.*?)( ?)$/; print quotemeta($1), $2'
+	))
 }
 
 __git_heads ()

-Peff
Previous: Jeff KingNext: SZEDER Gábor
Message 22 of 37 in “completion: fix shell expansion of items”
  1. completion: fix shell expansion of itemsFelipe Contreras, Sep 20, 2012
  2. Jeff KingSep 20, 2012
  3. Junio C HamanoSep 20, 2012
  4. SZEDER GáborSep 20, 2012
  5. Jeff KingSep 20, 2012
  6. Felipe ContrerasSep 20, 2012
  7. Revert "completion: fix shell expansion of items"Jeff King, Sep 25, 2012
  8. Junio C HamanoSep 25, 2012
  9. SZEDER GáborSep 25, 2012
  10. Junio C HamanoSep 25, 2012
  11. Jeff KingSep 26, 2012
  12. 1/3 t9902: add a few basic completion testsJeff King, Sep 26, 2012
  13. 2/3 t9902: add completion tests for "odd" filenamesJeff King, Sep 26, 2012
  14. 3/3 completion: improve shell expansion of itemsJeff King, Sep 26, 2012
  15. 4/3 completion: quote completions we findJeff King, Sep 26, 2012
  16. SZEDER GáborSep 27, 2012
  17. Junio C HamanoSep 27, 2012
  18. SZEDER GáborSep 27, 2012
  19. SZEDER GáborSep 27, 2012
  20. Jeff KingSep 27, 2012
  21. Jeff KingSep 27, 2012
  22. Jeff KingSep 27, 2012
  23. SZEDER GáborSep 28, 2012
  24. 1/5 completion: fix non-critical bugs in __gitcomp() testsSZEDER Gábor, Sep 28, 2012
  25. 2/5 completion: fix args of run_completion() test helperSZEDER Gábor, Sep 28, 2012
  26. Junio C HamanoSep 28, 2012
  27. SZEDER GáborSep 28, 2012
  28. Junio C HamanoSep 28, 2012
  29. Jeff KingSep 28, 2012
  30. Junio C HamanoSep 28, 2012
  31. Jeff KingSep 28, 2012
  32. SZEDER GáborSep 28, 2012
  33. 3/5 completion: add tests for the __gitcomp_nl() completion helper functionSZEDER Gábor, Sep 28, 2012
  34. 4/5 completion: test __gitcomp() and __gitcomp_nl() with expandable wordsSZEDER Gábor, Sep 28, 2012
  35. 5/5 completion: avoid compgen to fix expansion issues in __gitcomp_nl()SZEDER Gábor, Sep 28, 2012
  36. SZEDER GáborSep 28, 2012
  37. Jeff KingSep 28, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.