[PATCH v2 0/4] completion: trivial cleanups and fixes

[PATCH v2 0/4] 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.

Felipe Contreras (4):
  completion: simplify __gitcomp_1
  completion: trivial simplification
  completion: add missing general options
  completion: improve 'git --exec-path' completion

 contrib/completion/git-completion.bash |   16 +++++++++-------
 t/t9902-completion.sh                  |   18 +++++++++++++++++-
 2 files changed, 26 insertions(+), 8 deletions(-)

-- 
1.7.10.1.g1f19b8.dirty

[PATCH v2 1/4] completion: simplify __gitcomp_1

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 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 v2 2/4] completion: trivial simplification

[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 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 v2 3/4] 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 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 3bbec79..f2075af 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -92,8 +92,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 --"
@@ -114,4 +116,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 v2 4/4] completion: improve 'git --exec-path' completion

[Felipe Contreras]

All other options that accept an argument are completed this way, plus,
the '--foo bar' format doesn't seem to work correctly at the moment.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    2 +-
 t/t9902-completion.sh                  |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6a8cf9f..efed24a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2638,7 +2638,7 @@ _git ()
 			--git-dir=
 			--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 f2075af..fd2ed3e 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -90,7 +90,7 @@ test_expect_success 'double dash "git" itself' '
 	--git-dir=
 	--bare Z
 	--version Z
-	--exec-path Z
+	--exec-path=
 	--html-path Z
 	--info-path Z
 	--work-tree=
@@ -119,7 +119,7 @@ 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 " &&
+	test_completion "git --exe" "--exec-path=" &&
 	test_completion "git --htm" "--html-path " &&
 	test_completion "git --pag" "--paginate " &&
 	test_completion "git --no-p" "--no-pager " &&
-- 
1.7.10.1.g1f19b8.dirty

Re: [PATCH v2 4/4] completion: improve 'git --exec-path' completion

[Jonathan Nieder]

Felipe Contreras wrote:

> All other options that accept an argument are completed this way, plus,
> the '--foo bar' format doesn't seem to work correctly at the moment.
[...]
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2638,7 +2638,7 @@ _git ()
>  			--git-dir=
>  			--bare
>  			--version
> -			--exec-path
> +			--exec-path=
>  			--html-path

Thanks.

"git --exec-path" means to print the name of the directory where git
stores its subcommands and other helpers.  I have no thoughts either
way about whether a user typing

	git --exec-p<TAB>

is more likely to be asking for the current exec-path or intending to
override it.

Hope that helps,
Jonathan

Re: [PATCH v2 4/4] completion: improve 'git --exec-path' completion

[Jonathan Nieder]

On Fri, Apr 13, 2012 at 01:08:45AM -0500, Jonathan Nieder wrote:
> Felipe Contreras wrote:

>> +++ b/contrib/completion/git-completion.bash
>> @@ -2638,7 +2638,7 @@ _git ()
>>  			--git-dir=
>>  			--bare
>>  			--version
>> -			--exec-path
>> +			--exec-path=
[...]
> Thanks.
>
> "git --exec-path" means to print the name of the directory where git
> stores its subcommands and other helpers.  I have no thoughts either
> way about whether a user typing
>
> 	git --exec-p<TAB>
>
> is more likely to be asking for the current exec-path or intending to
> override it.

In other words, how about something like this?  Tests left as an exercise
to the interested reader.

-- >8 --
Subject: completion: do not add trailing space when completing --exec-path

--exec-path looks like to the completion script like an unambiguous
successful completion script, 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.

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 31f714da..ecbf2172 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -307,12 +307,14 @@ __git_ps1 ()
 # __gitcomp_1 requires 2 arguments
 __gitcomp_1 ()
 {
-	local c IFS=' '$'\t'$'\n'
+	local c word 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 " ;;
+		word=$c$2
+		case $word in
+		--*=*) printf %s$'\n' "$word" ;;
+		*.)    printf %s$'\n' "$word" ;;
+		*\$)   printf %s$'\n' "${word%\$}" ;;
+		*)     printf %s$'\n' "$word " ;;
 		esac
 	done
 }
@@ -2638,7 +2640,7 @@ _git ()
 			--git-dir=
 			--bare
 			--version
-			--exec-path
+			--exec-path\$
 			--html-path
 			--work-tree=
 			--namespace=
-- 
1.7.10

Re: [PATCH v2 4/4] completion: improve 'git --exec-path' completion

[Jonathan Nieder]

Jonathan Nieder wrote:

> In other words, how about something like this?  Tests left as an exercise
> to the interested reader.

... and here's a simpler way to spell it.

-- >8 --
Subject: completion: do not add trailing space when completing --exec-path

--exec-path looks like 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.

Reported-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 contrib/completion/git-completion.bash |    1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 31f714da..d2109897 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
 			--work-tree=
 			--namespace=
-- 
1.7.10

Re: [PATCH v2 4/4] completion: improve 'git --exec-path' completion

[Felipe Contreras]

On Fri, Apr 13, 2012 at 9:30 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Jonathan Nieder wrote:
>
>> In other words, how about something like this?  Tests left as an exercise
>> to the interested reader.
>
> ... and here's a simpler way to spell it.
>
> -- >8 --
> Subject: completion: do not add trailing space when completing --exec-path
>
> --exec-path looks like 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.
>
> Reported-by: Felipe Contreras <felipe.contreras@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  contrib/completion/git-completion.bash |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 31f714da..d2109897 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
>                        --work-tree=
>                        --namespace=
> --
> 1.7.10

I don't understand, the commit message doesn't match what the patch
actually does. In fact, this is the exact patch I sent, is it not?

-- 
Felipe Contreras

Re: [PATCH v2 4/4] completion: improve 'git --exec-path' completion

[Jonathan Nieder]

Felipe Contreras wrote:

> I don't understand, the commit message doesn't match what the patch
> actually does.

Try it, I guess?  At least for me,

	git --exec-p<TAB>

completes to

	git --exec-path <cursor here>

before the patch, and to

	git --exec-path<cursor here>

after the patch.  Which is different from your original patch.

Cheers,
Jonathan

Re: [PATCH v2 4/4] completion: improve 'git --exec-path' completion

[Felipe Contreras]

On Sat, Apr 14, 2012 at 2:37 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> I don't understand, the commit message doesn't match what the patch
>> actually does.
>
> Try it, I guess?  At least for me,
>
>        git --exec-p<TAB>
>
> completes to
>
>        git --exec-path <cursor here>
>
> before the patch, and to
>
>        git --exec-path<cursor here>
>
> after the patch.  Which is different from your original patch.

Ah, it doesn't remove the current line, it just adds a new one. I'm
not sure how the completion results show up, but I guess it's better
than the alternatives.

-- 
Felipe Contreras

Re: [PATCH v2 4/4] completion: improve 'git --exec-path' completion

[Andreas Schwab]

Jonathan Nieder <jrnieder@gmail.com> writes:

> --exec-path looks like to the completion script like an unambiguous

s/like (.*like)/\1/

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."