threads / patch / 31815

patchcompletion: add format-patch options to send-email

Subject: [PATCH] completion: add format-patch options to send-email

## tl;dr

6 messages between Oct 14, 2012 and Oct 15, 2012. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Felipe Contreras· Oct 14, 2012, 16:14 UTC · lore
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 35 +++++++++++++++++-----------------
 t/t9902-completion.sh                  | 21 ++++++++++++++++++++
 2 files changed, 39 insertions(+), 17 deletions(-)
Show changes to 2 files +39 −17

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d743e56..2a83504 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1112,6 +1112,14 @@ _git_fetch ()
 	__git_complete_remote_or_refspec
 }
 
+__git_format_patch_options="
+	--stdout --attach --no-attach --thread --thread= --output-directory
+	--numbered --start-number --numbered-files --keep-subject --signoff
+	--signature --no-signature --in-reply-to= --cc= --full-index --binary
+	--not --all --cover-letter --no-prefix --src-prefix= --dst-prefix=
+	--inline --suffix= --ignore-if-in-upstream --subject-prefix=
+"
+
 _git_format_patch ()
 {
 	case "$cur" in
@@ -1122,21 +1130,7 @@ _git_format_patch ()
 		return
 		;;
 	--*)
-		__gitcomp "
-			--stdout --attach --no-attach --thread --thread=
-			--output-directory
-			--numbered --start-number
-			--numbered-files
-			--keep-subject
-			--signoff --signature --no-signature
-			--in-reply-to= --cc=
-			--full-index --binary
-			--not --all
-			--cover-letter
-			--no-prefix --src-prefix= --dst-prefix=
-			--inline --suffix= --ignore-if-in-upstream
-			--subject-prefix=
-			"
+		__gitcomp "$__git_format_patch_options"
 		return
 		;;
 	esac
@@ -1550,6 +1544,12 @@ _git_send_email ()
 		__gitcomp "ssl tls" "" "${cur##--smtp-encryption=}"
 		return
 		;;
+	--thread=*)
+		__gitcomp "
+			deep shallow
+			" "" "${cur##--thread=}"
+		return
+		;;
 	--*)
 		__gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
 			--compose --confirm= --dry-run --envelope-sender
@@ -1559,11 +1559,12 @@ _git_send_email ()
 			--signed-off-by-cc --smtp-pass --smtp-server
 			--smtp-server-port --smtp-encryption= --smtp-user
 			--subject --suppress-cc= --suppress-from --thread --to
-			--validate --no-validate"
+			--validate --no-validate
+			$__git_format_patch_options"
 		return
 		;;
 	esac
-	COMPREPLY=()
+	__git_complete_revlist_file
 }
 
 _git_stage ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 92d7eb4..c4b6c13 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -146,6 +146,22 @@ test_expect_success '__gitcomp - suffix' '
 	test_cmp expected out
 '
 
+setup_repository ()
+{
+	mkdir "$1" && (
+	cd "$1" &&
+	git init &&
+	test_tick &&
+	git commit --allow-empty -m "Initial"
+	)
+}
+
+test_expect_success 'prepare' '
+	setup_repository one &&
+	git clone one test &&
+	cd test
+'
+
 test_expect_success 'basic' '
 	run_completion "git \"\"" &&
 	# built-in
@@ -228,4 +244,9 @@ test_expect_success 'general options plus command' '
 	test_completion "git --no-replace-objects check" "checkout "
 '
 
+test_expect_success 'send-email' '
+	test_completion "git send-email --cov" "--cover-letter " &&
+	test_completion "git send-email ma" "master "
+'
+
 test_done

-- 
1.7.12.1
SZEDER Gábor· Oct 15, 2012, 22:48 UTC · re: Felipe Contreras · lore

Re: [PATCH] completion: add format-patch options to send-email

Hi,
On Sun, Oct 14, 2012 at 06:14:03PM +0200, Felipe Contreras wrote:
Show 10 quoted lines
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 35 +++++++++++++++++-----------------
>  t/t9902-completion.sh                  | 21 ++++++++++++++++++++
>  2 files changed, 39 insertions(+), 17 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index d743e56..2a83504 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
This patch doesn't apply on current master.
Show 12 quoted lines
> @@ -1559,11 +1559,12 @@ _git_send_email ()
>  			--signed-off-by-cc --smtp-pass --smtp-server
>  			--smtp-server-port --smtp-encryption= --smtp-user
>  			--subject --suppress-cc= --suppress-from --thread --to
> -			--validate --no-validate"
> +			--validate --no-validate
> +			$__git_format_patch_options"
>  		return
>  		;;
>  	esac
> -	COMPREPLY=()
> +	__git_complete_revlist_file

While send-email accepts a rev-list, it doesn't accept 'HEAD:Documentation', does it? So __git_complete_revlist() would be better here, because that makes the intent clear.

Show 24 quoted lines
>  }
>  
>  _git_stage ()
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 92d7eb4..c4b6c13 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -146,6 +146,22 @@ test_expect_success '__gitcomp - suffix' '
>  	test_cmp expected out
>  '
>  
> +setup_repository ()
> +{
> +	mkdir "$1" && (
> +	cd "$1" &&
> +	git init &&
> +	test_tick &&
> +	git commit --allow-empty -m "Initial"
> +	)
> +}
> +
> +test_expect_success 'prepare' '
> +	setup_repository one &&
> +	git clone one test &&
Why are these new repositories needed?
> +	cd test

This 'cd' makes all subsequent tests to be executed in a different repository than previously, which breaks 'checkout completes ref names'.

Show 18 quoted lines
> +'
> +
>  test_expect_success 'basic' '
>  	run_completion "git \"\"" &&
>  	# built-in
> @@ -228,4 +244,9 @@ test_expect_success 'general options plus command' '
>  	test_completion "git --no-replace-objects check" "checkout "
>  '
>  
> +test_expect_success 'send-email' '
> +	test_completion "git send-email --cov" "--cover-letter " &&
> +	test_completion "git send-email ma" "master "
> +'
> +
>  test_done
> -- 
> 1.7.12.1
>
Junio C Hamano· Oct 15, 2012, 23:28 UTC · re: SZEDER Gábor · lore

Re: [PATCH] completion: add format-patch options to send-email

SZEDER Gábor <szeder@ira.uka.de> writes:
Show 5 quoted lines
>> +	cd test
>
> This 'cd' makes all subsequent tests to be executed in a different
> repository than previously, which breaks 'checkout completes ref
> names'.
Yeah, thanks for spotting and yelling at it ;-).

We really need to be careful about tests that cd around. Even the ones that only go down and expect the rest of the test to run in that subdirectory may later have to be modified by somebody to run back at the original level, and writing "cd ..", which leads to all sorts of problems, becomes too tempting.

Show 18 quoted lines
>> +'
>> +
>>  test_expect_success 'basic' '
>>  	run_completion "git \"\"" &&
>>  	# built-in
>> @@ -228,4 +244,9 @@ test_expect_success 'general options plus command' '
>>  	test_completion "git --no-replace-objects check" "checkout "
>>  '
>>  
>> +test_expect_success 'send-email' '
>> +	test_completion "git send-email --cov" "--cover-letter " &&
>> +	test_completion "git send-email ma" "master "
>> +'
>> +
>>  test_done
>> -- 
>> 1.7.12.1
>>
Felipe Contreras· Oct 15, 2012, 23:37 UTC · re: SZEDER Gábor · lore

Re: [PATCH] completion: add format-patch options to send-email

On Tue, Oct 16, 2012 at 12:48 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 16 quoted lines
>> @@ -1559,11 +1559,12 @@ _git_send_email ()
>>                       --signed-off-by-cc --smtp-pass --smtp-server
>>                       --smtp-server-port --smtp-encryption= --smtp-user
>>                       --subject --suppress-cc= --suppress-from --thread --to
>> -                     --validate --no-validate"
>> +                     --validate --no-validate
>> +                     $__git_format_patch_options"
>>               return
>>               ;;
>>       esac
>> -     COMPREPLY=()
>> +     __git_complete_revlist_file
>
> While send-email accepts a rev-list, it doesn't accept
> 'HEAD:Documentation', does it?  So __git_complete_revlist() would be
> better here, because that makes the intent clear.

Then _git_send_email should be fixed first. I'm simply doing the same as _git_send_email.

Show 23 quoted lines
>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
>> index 92d7eb4..c4b6c13 100755
>> --- a/t/t9902-completion.sh
>> +++ b/t/t9902-completion.sh
>> @@ -146,6 +146,22 @@ test_expect_success '__gitcomp - suffix' '
>>       test_cmp expected out
>>  '
>>
>> +setup_repository ()
>> +{
>> +     mkdir "$1" && (
>> +     cd "$1" &&
>> +     git init &&
>> +     test_tick &&
>> +     git commit --allow-empty -m "Initial"
>> +     )
>> +}
>> +
>> +test_expect_success 'prepare' '
>> +     setup_repository one &&
>> +     git clone one test &&
>
> Why are these new repositories needed?
Because otherwise 'git send-email ma' wouldn't succeed.
Show 5 quoted lines
>> +     cd test
>
> This 'cd' makes all subsequent tests to be executed in a different
> repository than previously, which breaks 'checkout completes ref
> names'.

I don't know which test you are talking about, it's not on my repo, and all the completion test pass with this patch.

Cheers.
-- 
Felipe Contreras
SZEDER Gábor· Oct 15, 2012, 23:55 UTC · re: Felipe Contreras · lore

Re: [PATCH] completion: add format-patch options to send-email

On Tue, Oct 16, 2012 at 01:37:35AM +0200, Felipe Contreras wrote:
Show 21 quoted lines
> On Tue, Oct 16, 2012 at 12:48 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> 
> >> @@ -1559,11 +1559,12 @@ _git_send_email ()
> >>                       --signed-off-by-cc --smtp-pass --smtp-server
> >>                       --smtp-server-port --smtp-encryption= --smtp-user
> >>                       --subject --suppress-cc= --suppress-from --thread --to
> >> -                     --validate --no-validate"
> >> +                     --validate --no-validate
> >> +                     $__git_format_patch_options"
> >>               return
> >>               ;;
> >>       esac
> >> -     COMPREPLY=()
> >> +     __git_complete_revlist_file
> >
> > While send-email accepts a rev-list, it doesn't accept
> > 'HEAD:Documentation', does it?  So __git_complete_revlist() would be
> > better here, because that makes the intent clear.
> 
> Then _git_send_email should be fixed first. I'm simply doing the same
> as _git_send_email.
I can't decipher this recursion here.
Show 25 quoted lines
> >> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> >> index 92d7eb4..c4b6c13 100755
> >> --- a/t/t9902-completion.sh
> >> +++ b/t/t9902-completion.sh
> >> @@ -146,6 +146,22 @@ test_expect_success '__gitcomp - suffix' '
> >>       test_cmp expected out
> >>  '
> >>
> >> +setup_repository ()
> >> +{
> >> +     mkdir "$1" && (
> >> +     cd "$1" &&
> >> +     git init &&
> >> +     test_tick &&
> >> +     git commit --allow-empty -m "Initial"
> >> +     )
> >> +}
> >> +
> >> +test_expect_success 'prepare' '
> >> +     setup_repository one &&
> >> +     git clone one test &&
> >
> > Why are these new repositories needed?
> 
> Because otherwise 'git send-email ma' wouldn't succeed.

Even then you don't need two additional repos, because just one commit in the test repo would suffice. And the test 'setup for ref completion' already takes care of that.

Show 8 quoted lines
> >> +     cd test
> >
> > This 'cd' makes all subsequent tests to be executed in a different
> > repository than previously, which breaks 'checkout completes ref
> > names'.
> 
> I don't know which test you are talking about, it's not on my repo,
> and all the completion test pass with this patch.

It's in v1.8.0-rc0~1^2 (t9902: add completion tests for "odd" filenames, 2012-09-26), which is the commit your patch conflicts with.

Best, Gábor

Felipe Contreras· Oct 15, 2012, 23:57 UTC · re: Felipe Contreras · lore

Re: [PATCH] completion: add format-patch options to send-email

On Tue, Oct 16, 2012 at 1:37 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

> On Tue, Oct 16, 2012 at 12:48 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 8 quoted lines
>>> +     cd test
>>
>> This 'cd' makes all subsequent tests to be executed in a different
>> repository than previously, which breaks 'checkout completes ref
>> names'.
>
> I don't know which test you are talking about, it's not on my repo,
> and all the completion test pass with this patch.
Maybe this then:
test_expect_success 'prepare' '
	git init &&
	test_tick &&
	git commit --allow-empty -m "Initial"
'

I originally created those for some push and fetch tests, but that required too many reorganizations, and I got fed up with the review, so I dropped the series[1].

Cheers.
[1] http://article.gmane.org/gmane.comp.version-control.git/197226
-- 
Felipe Contreras

← back to recent threads