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

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

[Felipe Contreras]

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

[PATCH v4 1/5] completion: simplify __gitcomp_1

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

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

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

[Felipe Contreras]

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(-)

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

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

[Felipe Contreras]

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(+)

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

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

[Felipe Contreras]

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(-)

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

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

[Felipe Contreras]

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(-)

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

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

[Junio C Hamano]

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.