On Fri, 24 Sept 2021 at 16:07, Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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).
