All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "SZEDER Gábor" <szeder@ira.uka.de>
Cc: Felipe Contreras <felipe.contreras@gmail.com>,
	git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 3/3] completion: improve shell expansion of items
Date: Wed, 26 Sep 2012 17:51:19 -0400	[thread overview]
Message-ID: <20120926215119.GC18653@sigill.intra.peff.net> (raw)
In-Reply-To: <20120926214653.GA18628@sigill.intra.peff.net>

The current completion code doesn't deal properly with items
(tags, branches, etc.) that have ${} in them because they
get expanded by bash while using compgen.

This patch is a rewrite of Felipe Contreras's 25ae7cf, which
attempted to fix the problem by quoting the values we pass
to __gitcomp_nl. However, that patch ended up quoting the
whole list as a single item, which broke other completions.
Instead, we need to split the newline-delimited list into
elements and quote each one individually.

This is not a complete fix, as the completion result does
will still contain metacharacters, so it would need extra
quoting to actually be used on a command line. But it's
still a step in the right direction, because:

  1. The current code's expansion means that things that are
     broken expansions (e.g., "${foo:bar}") will actually
     cause the completion function to barf, breaking all
     completion (even if you weren't going to complete that
     item). This patch fixes that so you can at least
     complete "foo" when "${foo:bar}" exists.

  2. We don't know yet what the final fix will look like,
     but this is probably the first step towards it. It
     handles quoting on the input side of compgen; the next
     step will likely be handling the quoting on the output
     side of compgen to yield a usable string for the
     command line.

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/completion/git-completion.bash | 14 +++++++++++++-
 t/t9902-completion.sh                  |  2 +-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index be800e0..b0416ea 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,6 +225,18 @@ fi
 fi
 fi
 
+# Quotes each element of an IFS-delimited list for shell reuse
+__git_quote()
+{
+	local i
+	local delim
+	for i in $1; do
+		local quoted=${i//\'/\'\\\'\'}
+		printf "${delim:+$IFS}'%s'" "$quoted"
+		delim=t
+	done
+}
+
 # Generates completion reply with compgen, appending a space to possible
 # completion words, if necessary.
 # It accepts 1 to 4 arguments:
@@ -261,7 +273,7 @@ __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}"))
 }
 
 __git_heads ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index cbd0fb6..da67705 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -278,7 +278,7 @@ test_expect_success 'complete tree filename with spaces' '
 	EOF
 '
 
-test_expect_failure 'complete tree filename with metacharacters' '
+test_expect_success 'complete tree filename with metacharacters' '
 	echo content >"name with \${meta}" &&
 	git add . &&
 	git commit -m meta &&
-- 
1.7.12.10.g31da6dd

  parent reply	other threads:[~2012-09-26 21:51 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-20  2:15 [PATCH] completion: fix shell expansion of items Felipe Contreras
2012-09-20  1:46 ` Jeff King
2012-09-20 16:52   ` Junio C Hamano
2012-09-20 18:11   ` SZEDER Gábor
2012-09-20 18:21     ` Jeff King
2012-09-20 19:38       ` Felipe Contreras
2012-09-25  4:31     ` [PATCH] Revert "completion: fix shell expansion of items" Jeff King
2012-09-25 16:03       ` Junio C Hamano
2012-09-25 22:37       ` SZEDER Gábor
2012-09-25 23:28         ` Junio C Hamano
2012-09-26 21:46         ` Jeff King
2012-09-26 21:47           ` [PATCH 1/3] t9902: add a few basic completion tests Jeff King
2012-09-26 21:51           ` [PATCH 2/3] t9902: add completion tests for "odd" filenames Jeff King
2012-09-26 21:51           ` Jeff King [this message]
2012-09-26 21:57             ` [PATCH 4/3] completion: quote completions we find Jeff King
2012-09-27 21:40               ` SZEDER Gábor
2012-09-27 22:31                 ` Junio C Hamano
2012-09-27 22:58                   ` SZEDER Gábor
2012-09-27  0:37             ` [PATCH 3/3] completion: improve shell expansion of items SZEDER Gábor
2012-09-27  6:28               ` Jeff King
2012-09-27  6:43                 ` Jeff King
2012-09-27 19:48                   ` Jeff King
2012-09-28 10:05                   ` SZEDER Gábor
2012-09-28 10:09                     ` [PATCH 1/5] completion: fix non-critical bugs in __gitcomp() tests SZEDER Gábor
2012-09-28 10:09                       ` [PATCH 2/5] completion: fix args of run_completion() test helper SZEDER Gábor
2012-09-28 18:04                         ` Junio C Hamano
2012-09-28 18:38                           ` SZEDER Gábor
2012-09-28 19:23                             ` Junio C Hamano
2012-09-28 19:30                               ` Jeff King
2012-09-28 19:49                                 ` Junio C Hamano
2012-09-28 19:55                                   ` Jeff King
2012-09-28 20:28                               ` SZEDER Gábor
2012-09-28 10:09                       ` [PATCH 3/5] completion: add tests for the __gitcomp_nl() completion helper function SZEDER Gábor
2012-09-28 10:09                       ` [PATCH 4/5] completion: test __gitcomp() and __gitcomp_nl() with expandable words SZEDER Gábor
2012-09-28 10:09                       ` [POC PATCH 5/5] completion: avoid compgen to fix expansion issues in __gitcomp_nl() SZEDER Gábor
2012-09-28 15:08                         ` SZEDER Gábor
2012-09-28 15:46                           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120926215119.GC18653@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=szeder@ira.uka.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.