Re: [PATCH v5 0/3] send-email: shell completion improvements

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 24, 2021, 20:02 UTC
Message-ID: <87bl4h3fgv.fsf@evledraar.gmail.com>
In-Reply-To: <20210924024606.20542-1-tbperrotta@gmail.com>

On Thu, Sep 23 2021, Thiago Perrotta wrote:

Show 18 quoted lines

> Thanks for the heads up, I am slowly getting used to this email
> workflow, this is my first contribution. Hopefully I got it right this
> time.
>
>> Isn't this just:
>
>> my @params = <getopts list>;
>> GetOptions(@params);
>
> Let me know if the regex approach I went with is OK. You seem to have
> suggested me to do something with the `GetOptions` module, but I'm
> afraid I only know the basics of Perl.
>
> I tried to do something like `GetOptions($USAGE)` but it didn't quite
> work (clearly I have no idea how to do that :P). If you have something
> specific in mind, I'd appreciate if you could send a small patch back
> that I can incorporate. Otherwise, either way, the current regex
> approach isn't too horrible and seems to be reasonably reliable.

I meant something like the below patch, feel free to incorporate it if you'd like with my signed-off-by, i.e. there's no reason to parse the usage message, or hardcode another set of options, we've got it right there as structured program data being fed to the GetOptions() function.

All we need to do is to assign that to a hash, and use it both for emitting the help and to call GetOptions().

What I have doesn't *quite* work, i.e. the --git-completion-helper expects "--foo=" I think for things that are "foo=s" in perl, so the regex needs adjusting, but that should be an easy addition on top.

-- >8 --

diff --git a/git-send-email.perl b/git-send-email.perl
index 5262d88ee32..221115fbbdd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -114,8 +114,18 @@ sub usage {
 }
 
 sub completion_helper {
-    print Git::command('format-patch', '--git-completion-helper');
-    exit(0);
+	my ($options) = @_;
+
+	my @gse_options = sort map {
+		"--$_"
+	} map {
+		s/(?:[:=][si]|!)$//;
+		split /\|/, $_;
+	} keys %$options;
+	my @fpa_options = Git::command('format-patch', '--git-completion-helper');
+	my @options = sort @gse_options, @fpa_options;
+	print "@options\n";
+	exit(0);
 }
 
 # most mail servers generate the Date: header, but not all...
@@ -449,7 +459,7 @@ sub config_regexp {
 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,
 		    "reply-to=s" => \$reply_to,
@@ -508,7 +518,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 +527,7 @@ sub config_regexp {
 my @initial_bcc = @getopt_bcc ? @getopt_bcc : ($no_bcc ? () : @config_bcc);
 
 usage() if $help;
-completion_helper() if $git_completion_helper;
+completion_helper(\%options) if $git_completion_helper;
 unless ($rc) {
     usage();
 }