threads / patch / 30251

v4, 5 partscompletion: trivial cleanups and fixes

Subject: [PATCH v4 0/5] completion: trivial cleanups and fixes

## tl;dr

7 messages between Apr 15, 2012 and Apr 15, 2012. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Felipe Contreras· Apr 15, 2012, 19:44 UTC · lore
Hi,
Just a few simpliciations, improvements, and add some missing options.
This series depends on the bash completion tests patch.
Since v3:
 * Fix potential issues with __gitcomp_1 and simplify even more
Since v2:
 * Add 'git --option <TAB>' fix
 * Improve --exec-path patch
Felipe Contreras (3):
  completion: simplify __gitcomp_1
  completion: simplify by using $prev
  completion: add missing general options
Jonathan Nieder (1):
  completion: avoid trailing space for --exec-path
SZEDER Gábor (1):
  completion: fix completion after 'git --option <TAB>'
 contrib/completion/git-completion.bash |   22 ++++++++++--------
 t/t9902-completion.sh                  |   38 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 9 deletions(-)
-- 
1.7.10.1.g1f19b8.dirty
Felipe Contreras· Apr 15, 2012, 19:44 UTC · re: Felipe Contreras · lore

[PATCH v4 1/5] completion: simplify __gitcomp_1

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
Show changes to contrib/completion/git-completion.bash +6 −6
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 31f714d..db2d3cc 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -304,16 +304,16 @@ __git_ps1 ()
 	fi
 }
 
-# __gitcomp_1 requires 2 arguments
 __gitcomp_1 ()
 {
-	local c IFS=' '$'\t'$'\n'
+	local c IFS=$' \t\n'
 	for c in $1; do
-		case "$c$2" in
-		--*=*) printf %s$'\n' "$c$2" ;;
-		*.)    printf %s$'\n' "$c$2" ;;
-		*)     printf %s$'\n' "$c$2 " ;;
+		c="$c$2"
+		case $c in
+		--*=*|*.) ;;
+		*) c="$c " ;;
 		esac
+		printf '%s\n' "$c"
 	done
 }
 
-- 
1.7.10.1.g1f19b8.dirty
Felipe Contreras· Apr 15, 2012, 19:44 UTC · re: Felipe Contreras · lore

[PATCH v4 2/5] completion: simplify by using $prev

cword-1 is the previous word ($prev).
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +2 −2
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index db2d3cc..8c91faf 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1658,7 +1658,7 @@ _git_notes ()
 		__gitcomp '--ref'
 		;;
 	,*)
-		case "${words[cword-1]}" in
+		case "$prev" in
 		--ref)
 			__gitcomp_nl "$(__git_refs)"
 			;;
@@ -1684,7 +1684,7 @@ _git_notes ()
 	prune,*)
 		;;
 	*)
-		case "${words[cword-1]}" in
+		case "$prev" in
 		-m|-F)
 			;;
 		*)
-- 
1.7.10.1.g1f19b8.dirty
Felipe Contreras· Apr 15, 2012, 19:44 UTC · re: Felipe Contreras · lore

[PATCH v4 3/5] completion: add missing general options

And add relevant tests.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    2 ++
 t/t9902-completion.sh                  |   16 ++++++++++++++++
 2 files changed, 18 insertions(+)
Show changes to 2 files +18 −0

contrib/completion/git-completion.bash, t/t9902-completion.sh

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8c91faf..b837704 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2640,8 +2640,10 @@ _git ()
 			--version
 			--exec-path
 			--html-path
+			--info-path
 			--work-tree=
 			--namespace=
+			--no-replace-objects
 			--help
 			"
 			;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7bd37f5..f24c968 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -95,8 +95,10 @@ test_expect_success 'double dash "git" itself' '
 	--version Z
 	--exec-path Z
 	--html-path Z
+	--info-path Z
 	--work-tree=
 	--namespace=
+	--no-replace-objects Z
 	--help Z
 	EOF
 	test_completion "git --"
@@ -117,4 +119,18 @@ test_expect_success 'double dash "git checkout"' '
 	test_completion "git checkout --"
 '
 
+test_expect_success 'general options' '
+	test_completion "git --ver" "--version " &&
+	test_completion "git --hel" "--help " &&
+	test_completion "git --exe" "--exec-path " &&
+	test_completion "git --htm" "--html-path " &&
+	test_completion "git --pag" "--paginate " &&
+	test_completion "git --no-p" "--no-pager " &&
+	test_completion "git --git" "--git-dir=" &&
+	test_completion "git --wor" "--work-tree=" &&
+	test_completion "git --nam" "--namespace=" &&
+	test_completion "git --bar" "--bare " &&
+	test_completion "git --inf" "--info-path " &&
+	test_completion "git --no-r" "--no-replace-objects "
+'
 test_done
-- 
1.7.10.1.g1f19b8.dirty
Felipe Contreras· Apr 15, 2012, 19:44 UTC · re: Felipe Contreras · lore

[PATCH v4 4/5] completion: avoid trailing space for --exec-path

From: Jonathan Nieder <jrnieder@gmail.com>

--exec-path looks to the completion script like an unambiguous successful completion, but it is wrong; the user could be trying to do

	git --exec-path; # print name of helper directory
or
	git --exec-path=/path/to/alternative/helper/dir <subcommand>

so the most helpful thing to do is to leave out the trailing space and leave it to the operator to type an equal sign or carriage return according to the situation.

Cc: Andreas Schwab <schwab@linux-m68k.org>
Reported-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
[added tests]
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    1 +
 t/t9902-completion.sh                  |    7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)
Show changes to 2 files +7 −1

contrib/completion/git-completion.bash, t/t9902-completion.sh

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b837704..6486a09 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2639,6 +2639,7 @@ _git ()
 			--bare
 			--version
 			--exec-path
+			--exec-path=
 			--html-path
 			--info-path
 			--work-tree=
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f24c968..dfef809 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -94,6 +94,7 @@ test_expect_success 'double dash "git" itself' '
 	--bare Z
 	--version Z
 	--exec-path Z
+	--exec-path=
 	--html-path Z
 	--info-path Z
 	--work-tree=
@@ -122,7 +123,11 @@ test_expect_success 'double dash "git checkout"' '
 test_expect_success 'general options' '
 	test_completion "git --ver" "--version " &&
 	test_completion "git --hel" "--help " &&
-	test_completion "git --exe" "--exec-path " &&
+	sed -e "s/Z$//" >expected <<-\EOF &&
+	--exec-path Z
+	--exec-path=
+	EOF
+	test_completion "git --exe" &&
 	test_completion "git --htm" "--html-path " &&
 	test_completion "git --pag" "--paginate " &&
 	test_completion "git --no-p" "--no-pager " &&
-- 
1.7.10.1.g1f19b8.dirty
Felipe Contreras· Apr 15, 2012, 19:44 UTC · re: Felipe Contreras · lore

[PATCH v4 5/5] completion: fix completion after 'git --option <TAB>'

From: SZEDER Gábor <szeder@ira.uka.de>

Git's bash completion currently doesn't work when certain git options are specified, e.g. 'git --no-pager <TAB>' errors out with "error: invalid key: alias.--no-pager".

The main _git() completion function finds out the git command name by looping through all the words on the command line and searching for the first word that is not a known option for the git command. Unfortunately the list of known git options was not updated in a long time, and newer options are not skipped but mistaken for a git command. Such a misrecognized "command" is then passed to __git_aliased_command(), which in turn passes it to a 'git config' query, hence the error.

Currently the following options are misrecognized for a git command:
  -c --no-pager --exec-path --html-path --man-path --info-path
  --no-replace-objects --work-tree= --namespace=

To fix this we could just update the list of options to be skipped, but the same issue will likely arise, if the git command learns a new option in the future. Therefore, to make it more future proof against new options, this patch changes that loop to skip all option-looking words, i.e. words starting with a dash.

We also have to handle the '-c' option specially, because it takes a configutation parameter in a separate word, which must be skipped, too.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
[added tests]
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    3 ++-
 t/t9902-completion.sh                  |   17 +++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)
Show changes to 2 files +19 −1

contrib/completion/git-completion.bash, t/t9902-completion.sh

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6486a09..9f56ec7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2623,8 +2623,9 @@ _git ()
 		case "$i" in
 		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
 		--bare)      __git_dir="." ;;
-		--version|-p|--paginate) ;;
 		--help) command="help"; break ;;
+		-c) c=$((++c)) ;;
+		-*) ;;
 		*) command="$i"; break ;;
 		esac
 		((c++))
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index dfef809..ab72378 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -138,4 +138,21 @@ test_expect_success 'general options' '
 	test_completion "git --inf" "--info-path " &&
 	test_completion "git --no-r" "--no-replace-objects "
 '
+
+test_expect_success 'general options plus command' '
+	test_completion "git --version check" "checkout " &&
+	test_completion "git --paginate check" "checkout " &&
+	test_completion "git --git-dir=foo check" "checkout " &&
+	test_completion "git --bare check" "checkout " &&
+	test_completion "git --help des" "describe " &&
+	test_completion "git --exec-path=foo check" "checkout " &&
+	test_completion "git --html-path check" "checkout " &&
+	test_completion "git --no-pager check" "checkout " &&
+	test_completion "git --work-tree=foo check" "checkout " &&
+	test_completion "git --namespace=foo check" "checkout " &&
+	test_completion "git --paginate check" "checkout " &&
+	test_completion "git --info-path check" "checkout " &&
+	test_completion "git --no-replace-objects check" "checkout "
+'
+
 test_done
-- 
1.7.10.1.g1f19b8.dirty
Junio C Hamano· Apr 15, 2012, 21:45 UTC · re: Felipe Contreras · lore

Re: [PATCH v4 0/5] completion: trivial cleanups and fixes

Felipe Contreras <felipe.contreras@gmail.com> writes:
> Just a few simpliciations, improvements, and add some missing options.
>
> This series depends on the bash completion tests patch.

It looks like it addresses more or less all the issues raised in earlier rounds. Will queue.

Thanks all.

← back to recent threads