"git send-email --git-completion-helper" only prints "format-patch"
flags. Make it print "send-email" flags as well, extracting them
programmatically from its three existing "GetOptions".
Introduce a "uniq" subroutine, otherwise --cc-cover, --to-cover and
other flags would show up twice. In addition, deduplicate flags common
to both "send-email" and "format-patch", like --from.
Remove extraneous flags: --, --h and --git-completion-helper.
Add a completion test for "send-email --validate", a send-email flag.
Signed-off-by: Thiago Perrotta <tbperrotta@xxxxxxxxx>
Based-on-patch-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
> ...I think that re-indentation is better left alone for the patch
> readability.
Reverted the `GetOptions` indentation. Noise is now gone :-)
> First, in your 1/3 you're adding a \n, but in 2/3 we end up with \n\n. I
> think you can just skip 1/3, maybe mention "how it also has a "\n" in
> the commit message.
I don't quite see how this would fit into the commit message. A comment in the
code seems to fit better to account for this detail. That's what I did, but if
you still disagree, please elaborate where in the commit message this sentence
should be added.
> You then strip out "--" arguments from the combined list, but isn't this
> something we do need to emit? I.e. it's used as syntax by the bash
> completion isn't it? (I just skimmed the relevant C code in
> parse-options.c).
I interpreted that standalone `--` as an extraneous / useless token. If it's
there intentionally, then I am reverting my stripping it. At the end of the day
whether to include it or not is a small detail, but FYI, when I do:
git clone -<TAB>
in bash, nothing happens. I would have expected it to be expanded to "--"
because of the explicit "--", but it doesn't. Therefore my conclusion is that
"--" in the output of "--git-completion-helper" is useless. Am I missing
something?
Finally, as per your comments about "=": OK, I see what you mean. For the record
I had produced a quick-and-dirty version earlier (on top of V6), but didn't
include it in V7 because I didn't deem it polished enough, but I'll include it
here for reference. I'll take your new comments to see if I can produce a better
polished version before I send V8:
diff --git a/git-send-email.perl b/git-send-email.perl
index 465e9867b9..f0f83330d3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -123,12 +123,13 @@ sub completion_helper {
my @send_email_opts = map {
"--$_"
} map {
- s/(?:[:=][si]|!)$//;
+ s/!$//;
+ s/[:=][si]$/=/;
split /\|/, $_;
} keys %$options;
- my @format_patch_opts = Git::command('format-patch', '--git-completion-helper');
- my @options = uniq @send_email_opts, @format_patch_opts;
- @options = grep !/--git-completion-helper|--h/, @options;
+ my @format_patch_opts = split(/ /, Git::command('format-patch', '--git-completion-helper'));
+ my @options = (@send_email_opts, @format_patch_opts);
+ @options = uniq (grep !/^$|^--$|--git-completion-helper|--h/, @options);
print "@options\n";
exit(0);
}
> That it's a map/map/keys is just some off-the-cuff Perl hacking on my
part, I think for validation etc. it's usually better to just turn it
into a plain old boring for-loop.
Oh, okay, I didn't find it hacky at all, the `map` seemed quite elegant and
functional to me, but then again I am a perl newbie :-P.
Differences from V7 (so far):
As per Ævar's comments:
- completely drop 1/3 since we'now using split instead of print to incorporate
format-patch options. Side effect: then-extraneous '\n' is now gone.
- revert indentation (tabs) on GetOptions to eliminate diff noise (as per Ævar's
interpretation of original Carlos's comment)
- re-add "--" to the output of "git send-email --git-completion-helper"
contrib/completion/git-completion.bash | 11 +------
git-send-email.perl | 40 ++++++++++++++++++++------
t/t9902-completion.sh | 3 ++
3 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4bdd27ddc8..1b73a4dcc0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2359,16 +2359,7 @@ _git_send_email ()
return
;;
--*)
- __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
- --no-suppress-from --no-thread --quiet --reply-to
- --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
- $__git_format_patch_extra_options"
+ __gitcomp_builtin send-email "$__git_format_patch_extra_options"
return
;;
esac
diff --git a/git-send-email.perl b/git-send-email.perl
index e65d969d0b..b632313192 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -113,9 +113,25 @@ sub usage {
exit(1);
}
+sub uniq {
+ my %seen;
+ grep !$seen{$_}++, @_;
+}
+
sub completion_helper {
- print Git::command('format-patch', '--git-completion-helper');
- exit(0);
+ my ($original_opts) = @_;
+ my @send_email_opts = map {
+ "--$_"
+ } map {
+ s/(?:[:=][si]|!)$//;
+ split /\|/;
+ } keys %$original_opts;
+ my @format_patch_opts = split(/ /, Git::command('format-patch', '--git-completion-helper'));
+ my @options = (@send_email_opts, @format_patch_opts);
+ @options = uniq (grep !/^$|--git-completion-helper|--h/, @options);
+ # There's an implicit '\n' here already, no need to add an explicit one.
+ print "@options";
+ exit(0);
}
# most mail servers generate the Date: header, but not all...
@@ -425,10 +441,11 @@ sub config_regexp {
my $key = "sendemail.identity";
$identity = Git::config(@repo, $key) if exists $known_config_keys{$key};
}
-my $rc = GetOptions(
+my %identity_options = (
"identity=s" => \$identity,
"no-identity" => \$no_identity,
);
+my $rc = GetOptions(%identity_options);
usage() unless $rc;
undef $identity if $no_identity;
@@ -444,14 +461,17 @@ sub config_regexp {
my $help;
my $git_completion_helper;
-$rc = GetOptions("h" => \$help,
- "dump-aliases" => \$dump_aliases);
+my %dump_aliases_options = (
+ "h" => \$help,
+ "dump-aliases" => \$dump_aliases,
+);
+$rc = GetOptions(%dump_aliases_options);
usage() unless $rc;
die __("--dump-aliases incompatible with other options\n")
if !$help and $dump_aliases and @ARGV;
-$rc = GetOptions(
+my %options = (
"sender|from=s" => \$sender,
- "in-reply-to=s" => \$initial_in_reply_to,
+ "in-reply-to=s" => \$initial_in_reply_to,
"reply-to=s" => \$reply_to,
"subject=s" => \$initial_subject,
"to=s" => \@getopt_to,
@@ -508,7 +528,8 @@ sub config_regexp {
"batch-size=i" => \$batch_size,
"relogin-delay=i" => \$relogin_delay,
"git-completion-helper" => \$git_completion_helper,
- );
+);
+$rc = GetOptions(%options);
# Munge any "either config or getopt, not both" variables
my @initial_to = @getopt_to ? @getopt_to : ($no_to ? () : @config_to);
@@ -516,7 +537,8 @@ sub config_regexp {
my @initial_bcc = @getopt_bcc ? @getopt_bcc : ($no_bcc ? () : @config_bcc);
usage() if $help;
-completion_helper() if $git_completion_helper;
+my %all_options = (%options, %dump_aliases_options, %identity_options);
+completion_helper(\%all_options) if $git_completion_helper;
unless ($rc) {
usage();
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 11573936d5..a4faf64184 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2139,6 +2139,9 @@ test_expect_success PERL 'send-email' '
--cover-from-description=Z
--cover-letter Z
EOF
+ test_completion "git send-email --val" <<-\EOF &&
+ --validate Z
+ EOF
test_completion "git send-email ma" "main "
'
--
2.33.0
