threads / patch / 30230

v2, 4 partscompletion: simplify __gitcomp_1

Subject: [PATCH v2 1/4] completion: simplify __gitcomp_1

## tl;dr

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

replies: 11people: 3as markdown or json

Felipe Contreras· Apr 12, 2012, 23:49 UTC · lore

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

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
Felipe Contreras· Apr 12, 2012, 23:50 UTC · re: Felipe Contreras · lore
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Show changes to contrib/completion/git-completion.bash +4 −4
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
Felipe Contreras· Apr 12, 2012, 23:50 UTC · re: Felipe Contreras · lore

[PATCH v2 2/4] completion: trivial simplification

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 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
Felipe Contreras· Apr 12, 2012, 23:50 UTC · re: Felipe Contreras · lore

[PATCH v2 3/4] 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 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
Felipe Contreras· Apr 12, 2012, 23:50 UTC · re: Felipe Contreras · lore

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

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(-)
Show changes to 2 files +3 −3

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

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
Jonathan Nieder· Apr 13, 2012, 06:08 UTC · re: Felipe Contreras · lore

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

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.
[...]
Show 9 quoted lines
> --- 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

Jonathan Nieder· Apr 13, 2012, 18:04 UTC · re: Jonathan Nieder · lore

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

On Fri, Apr 13, 2012 at 01:08:45AM -0500, Jonathan Nieder wrote:
> Felipe Contreras wrote:
Show 7 quoted lines
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2638,7 +2638,7 @@ _git ()
>>  			--git-dir=
>>  			--bare
>>  			--version
>> -			--exec-path
>> +			--exec-path=
[...]
Show 10 quoted lines
> 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(-)
Show changes to contrib/completion/git-completion.bash +8 −6
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
Jonathan Nieder· Apr 13, 2012, 18:30 UTC · re: Jonathan Nieder · lore

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

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(+)
Show changes to contrib/completion/git-completion.bash +1 −0
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
Felipe Contreras· Apr 13, 2012, 23:35 UTC · re: Jonathan Nieder · lore

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

On Fri, Apr 13, 2012 at 9:30 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 44 quoted lines
> 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
Jonathan Nieder· Apr 13, 2012, 23:37 UTC · re: Felipe Contreras · lore

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

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

Felipe Contreras· Apr 13, 2012, 23:43 UTC · re: Jonathan Nieder · lore

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

On Sat, Apr 14, 2012 at 2:37 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 18 quoted lines
> 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
Andreas Schwab· Apr 14, 2012, 07:21 UTC · re: Jonathan Nieder · lore

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

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."

← back to recent threads