From: Duy Nguyen <pclouds@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Denton Liu" <liu.denton@gmail.com>,
"SZEDER Gábor" <szeder.dev@gmail.com>,
"Git Mailing List" <git@vger.kernel.org>,
"Anmol Mago" <anmolmago@gmail.com>,
briankyho@gmail.com, david.lu97@outlook.com,
shirui.wang@hotmail.com
Subject: Re: [PATCH v2] completion: use builtin completion for format-patch
Date: Sat, 3 Nov 2018 07:03:18 +0100 [thread overview]
Message-ID: <20181103060317.GA5432@duynguyen.home> (raw)
In-Reply-To: <xmqqzhuss6dd.fsf@gitster-ct.c.googlers.com>
On Fri, Nov 02, 2018 at 08:52:30AM +0900, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> 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@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
@@ -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
next prev parent reply other threads:[~2018-11-03 6:03 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-29 17:57 [PATCH] completion: use builtin completion for format-patch Denton Liu
2018-10-30 2:20 ` Junio C Hamano
2018-10-30 3:50 ` Denton Liu
2018-10-30 6:38 ` [PATCH v2] " Denton Liu
2018-10-30 7:33 ` Junio C Hamano
2018-10-30 15:29 ` Duy Nguyen
2018-11-01 1:42 ` Junio C Hamano
2018-11-01 15:40 ` Duy Nguyen
2018-11-01 23:52 ` Junio C Hamano
2018-11-03 6:03 ` Duy Nguyen [this message]
2018-11-03 7:59 ` Denton Liu
2018-11-03 8:29 ` Duy Nguyen
2018-11-03 10:20 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181103060317.GA5432@duynguyen.home \
--to=pclouds@gmail.com \
--cc=anmolmago@gmail.com \
--cc=briankyho@gmail.com \
--cc=david.lu97@outlook.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=liu.denton@gmail.com \
--cc=shirui.wang@hotmail.com \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).