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

On Fri, 24 Sept 2021 at 16:07, Ævar Arnfjörð Bjarmason [off-list ref] wrote:

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.

1)
Thanks Ævar, I get the gist of it. Your approach revealed a few issues
with the current usage string:

The following options exist in GetOptions but not in the usage string:

--git-completion-helper
--no-signed-off-cc
--sender
--signed-off-cc

Out of these, I'd argue --git-completion-helper is intentionally omitted,
however --sender and --signed-off-cc were overlooked.

2)
Also, your patch misses --dump-aliases and --identity; that's because
they are in other GetOptions functions in the file.

The two obvious possibilities here are either (i) hard-code them directly, i.e.:

-my @options = sort @gse_options, @fpa_options;
+my @options = sort @gse_options, @fpa_options, "--dump-aliases", "--sender";

or (ii) refactor the other two GetOptions like you did in your patch,
so that `sub completion_helper` ends up receiving all three hashes
(or maybe a single hash as a result of all three merged).

Any preference between (i) or (ii)? I am leaning towards (i).

3)
Finally, I noticed that "sort @gse_options, @fpa_options" doesn't
really sort fpa_options.

If sorting is really intended, it would be better to modify the source
of format-patch to
emit sorted output.

Otherwise, we may as well leave it untouched. AFAIK from a completion
perspective it
seems that it doesn't matter: both bash and zsh emit `git format-patch
--<TAB>` sorted
today, even though the output of `git format-patch
--git-completion-helper` isn't sorted.
The only benefit of sorting I see would be to deduplicate ('uniq') flags.

Do you agree with this rationale?
Either way, let me know whether or not it's preferable to sort.
I'll probably sort `send-email` options anyway just to deduplicate a
few flags such as --to-cover,
but `format-patch` could remain as is.


I'll wait for replies before sending another patch (on top of your
original one).