On Sat, Nov 03, 2018 at 07:03:18AM +0100, Duy Nguyen wrote:
> 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.den...@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@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.

> @@ -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< --
> --
> Duy

Aside from that one comment, it looks good to me. Thanks for helping me
clean up my earlier patch!

Reply via email to