Re: [PATCH v6 2/3] send-email: programmatically generate bash completions

["Carlo Marcelo Arenas Belón" <carenas@gmail.com>]

On Wed, Oct 06, 2021 at 11:36:51PM -0400, Thiago Perrotta wrote:
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d1731c1755..465e9867b9 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -40,7 +40,7 @@ package main;
>  
>  sub usage {
>  	print <<EOT;
> -git send-email [options] <file | directory | rev-list options >
> +git send-email [options] <file | directory | rev-list options>

this change isn't really needed now that you are not using the usage()
to get the options, and should probably go into an independent patch,
or even better, as part of patch 3 so it is consistent.

> @@ -113,8 +113,23 @@ sub usage {
>  	exit(1);
>  }
>  
> +sub uniq {
> +    my %seen;
> +    grep !$seen{$_}++, @_;
> +}

indentation in this file is a little odd, but will be better if you use the
most common one (tab), instead of your own version here (4 or 2 spaces) and
in the rest of the code.

>  sub completion_helper {
> -    print Git::command('format-patch', '--git-completion-helper'), "\n";
> +    my ($options) = @_;
> +    my @send_email_opts = map {
> +      "--$_"
> +    } map {
> +      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;

reusing "options" here makes this code more confusing than it needs to be
maybe something like :

diff --git a/git-send-email.perl b/git-send-email.perl
index a1338dd774..d47543bc0d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -119,13 +119,13 @@ sub uniq {
 }

 sub completion_helper {
-    my ($options) = @_;
+    my ($original_opts) = @_;
     my @send_email_opts = map {
       "--$_"
     } map {
       s/(?:[:=][si]|!)$//;
-      split /\|/, $_;
-    } keys %$options;
+      split /\|/;
+    } keys %$original_opts;
     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;

might help on top

Carlo