On Fri, Nov 02, 2018 at 08:52:30AM +0900, Junio C Hamano wrote: > Duy Nguyen <pclouds@xxxxxxxxx> writes: > > >> > I have no comment about this. In an ideal world, sendemail.perl could > >> > be taught to support --git-completion-helper but I don't think my > >> > little remaining Perl knowledge (or time) is enough to do it. Perhaps > >> > this will do. I don't know. > >> > >> So "all", "attach", etc. are added to this list while these similar > >> options are lost from the other variable? Is this a good trade-off? > > > > Not sure if I understand you correctly, but it looks to me that the > > options in git-send-email.perl are well organized, so we could... > > Yes, but I wasn't commenting on your "sendemail should also be able > to help completion by supporting --completion-helper option" (I think > that is a sensible approach). My comment was about Denton's patch, > which reduced the hard-coded list of format-patch options (i.e. the > first hunk) but had to add back many of them to send-email's > completion (i.e. the last hunk)---overall, it did not help reducing > the number of options hardcoded in the script. > > If it makes sense to complete all options to format-patch to > send-email, then as you outlined, grabbing them out of format-patch > with the --completion-helper option at runtime, and using them to > complete both format-patch and send-email would be a good idea. And > that should be doable even before send-email learns how to list its > supported options to help the completion. OK how about this? Minimal changes in send-email.perl and no duplication in _git_send_email(). I could do $(git format-patch --git-completion-helper) directly from _git_send_email() too but we lose caching. -- 8< -- 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@xxxxxxxxx> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> --- 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 @@ -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