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

["SZEDER Gábor" <szeder@ira.uka.de>]

On Tue, Oct 16, 2012 at 01:37:35AM +0200, Felipe Contreras wrote:
> 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.

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

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