Re: [PATCH v2] completion: use builtin completion for format-patch
- From
Denton Liu <liu.denton@gmail.com>
- Date
- Nov 3, 2018, 07:59 UTC
- Message-ID
- <20181103075918.GA905@archbookpro.localdomain>
- In-Reply-To
- <20181103060317.GA5432@duynguyen.home>
On Sat, Nov 03, 2018 at 07:03:18AM +0100, Duy Nguyen wrote:
Show 65 quoted lines
> Subject: [PATCH] completion: use __gitcomp_builtin for format-patch > > This helps format-patch gain completion for a couple new options, > notably --range-diff. > > Since send-email completion relies on $__git_format_patch_options > which is now reduced, we need to do something not to regress > send-email completion. > > The workaround here is implement --git-completion-helper in > send-email.perl just as a bridge to "format-patch --git-completion-helper". > This is enough to use __gitcomp_builtin on send-email (to take > advantage of caching). > > In the end, send-email.perl can probably reuse the same info it passes > to GetOptions() to generate full --git-completion-helper output so > that we don't need to keep track of its options in git-completion.bash > anymore. But that's something for another boring day. > > Helped-by: Denton Liu <liu.denton@gmail.com> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > contrib/completion/git-completion.bash | 16 ++++++---------- > git-send-email.perl | 8 ++++++++ > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index db7fd87b6b..8409978793 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1532,13 +1532,9 @@ _git_fetch () > __git_complete_remote_or_refspec > } > > -__git_format_patch_options=" > - --stdout --attach --no-attach --thread --thread= --no-thread > - --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= > - --output-directory --reroll-count --to= --quiet --notes > +__git_format_patch_extra_options=" > + --full-index --not --all --no-prefix --src-prefix= > + --dst-prefix= --notes > " > > _git_format_patch () > @@ -1551,7 +1547,7 @@ _git_format_patch () > return > ;; > --*) > - __gitcomp "$__git_format_patch_options" > + __gitcomp_builtin format-patch "$__git_format_patch_extra_options" > return > ;; > esac > @@ -2081,7 +2077,7 @@ _git_send_email () > return > ;; > --*) > - __gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to > + __gitcomp_builtin send-email "--annotate --bcc --cc --cc-cmd --chain-reply-to > --compose --confirm= --dry-run --envelope-sender > --from --identity > --in-reply-to --no-chain-reply-to --no-signed-off-by-cc
Would it make sense to make send-email's completion helper print these out directly? That way, if someone were to modify send-email in the future, they'd only have to look through one file instead of both send-email and the completions script.
Show 51 quoted lines
> @@ -2090,7 +2086,7 @@ _git_send_email ()
> --smtp-server-port --smtp-encryption= --smtp-user
> --subject --suppress-cc= --suppress-from --thread --to
> --validate --no-validate
> - $__git_format_patch_options"
> + $__git_format_patch_extra_options"
> return
> ;;
> esac
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 2be5dac337..ed0714eaaa 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -119,6 +119,11 @@ sub usage {
> exit(1);
> }
>
> +sub completion_helper {
> + print Git::command('format-patch', '--git-completion-helper');
> + exit(0);
> +}
> +
> # most mail servers generate the Date: header, but not all...
> sub format_2822_time {
> my ($time) = @_;
> @@ -311,6 +316,7 @@ sub signal_handler {
> # needing, first, from the command line:
>
> my $help;
> +my $git_completion_helper;
> my $rc = GetOptions("h" => \$help,
> "dump-aliases" => \$dump_aliases);
> usage() unless $rc;
> @@ -373,9 +379,11 @@ sub signal_handler {
> "no-xmailer" => sub {$use_xmailer = 0},
> "batch-size=i" => \$batch_size,
> "relogin-delay=i" => \$relogin_delay,
> + "git-completion-helper" => \$git_completion_helper,
> );
>
> usage() if $help;
> +completion_helper() if $git_completion_helper;
> unless ($rc) {
> usage();
> }
> --
> 2.19.1.1005.gac84295441
>
> -- 8< --
> --
> DuyAside from that one comment, it looks good to me. Thanks for helping me clean up my earlier patch!