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

DORMANTno replies

Revision v3 of 3 in this series.

8 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:35

Hi,

Just a few simpliciations, improvements, and add some missing options.

This series depends on the bash completion tests patch.

Since v2:

 * Add 'git --option <TAB>' fix
 * Improve --exec-path patch

Felipe Contreras (3):
  completion: simplify __gitcomp_1
  completion: trivial simplification
  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 |   18 +++++++++------
 t/t9902-completion.sh                  |   38 ++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

-- 
1.7.10.1.g1f19b8.dirty

[PATCH v3 1/5] completion: simplify __gitcomp_1

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:35

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/completion/git-completion.bash |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 31f714d..13be9a7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -307,13 +307,13 @@ __git_ps1 ()
 # __gitcomp_1 requires 2 arguments
 __gitcomp_1 ()
 {
-	local c IFS=' '$'\t'$'\n'
+	local c s 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 " ;;
+		--*=* | *.) s="" ;;
+		*)          s=" " ;;
 		esac
+		echo "$c$2$s"
 	done
 }
 
-- 
1.7.10.1.g1f19b8.dirty

[PATCH v3 2/5] completion: trivial simplification

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:35

cword-1 is the previous word ($prev).

Signed-off-by: Felipe Contreras <redacted>
---
 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 13be9a7..9d36bb7 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 v3 3/5] completion: add missing general options

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:35

And add relevant tests.

Signed-off-by: Felipe Contreras <redacted>
---
 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 9d36bb7..6a8cf9f 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 v3 4/5] completion: avoid trailing space for --exec-path

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:35

From: Jonathan Nieder <redacted>

--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 <redacted>
Reported-by: Felipe Contreras <redacted>
Signed-off-by: Jonathan Nieder <redacted>
[added tests]
Signed-off-by: Felipe Contreras <redacted>
---
 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 6a8cf9f..647ee77 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 v3 5/5] completion: fix completion after 'git --option <TAB>'

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:35

From: SZEDER Gábor <redacted>

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 <redacted>
[added tests]
Signed-off-by: Felipe Contreras <redacted>
---
 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 647ee77..192e444 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 v3 0/5] completion: trivial cleanups and fixes

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:35

Felipe Contreras wrote:
Just a few simpliciations, improvements, and add some missing options.
I generally like this.  Quick comments:
Felipe Contreras (3):
  completion: simplify __gitcomp_1
Thomas and Gábor seem to have this one covered, with some improvements
to it under discussion.
  completion: trivial simplification
Improves consistency.  The changelog line isn't precise enough to
remind me which change is being mentioned when reading it in the
shortlog, so I guess I'd prefer something like

	completion: simplify by using $prev variable
  completion: add missing general options
Likewise, a subject line like the following would work better for
me.

	completion: --info-path and --no-replace-objects options for git

Hope that helps,
Jonathan

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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:35

On Sun, Apr 15, 2012 at 4:20 PM, Jonathan Nieder [off-list ref] wrote:
Felipe Contreras wrote:
quoted
  completion: trivial simplification
Improves consistency.  The changelog line isn't precise enough to
remind me which change is being mentioned when reading it in the
shortlog, so I guess I'd prefer something like

       completion: simplify by using $prev variable
OK.
quoted
  completion: add missing general options
Likewise, a subject line like the following would work better for
me.

       completion: --info-path and --no-replace-objects options for git
That's too long, and I think it's less understandable: for git? Of
course it's for git, for what else could it be? :) maybe s/options for
git/general options/, but it's still too long.

I'll leave it like that, anybody else can change it if they wish.

Cheers.

-- 
Felipe Contreras
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help