Re: [PATCH] send-email: avoid duplicate specification warnings

[Jeff King <peff@peff.net>]

On Tue, Nov 14, 2023 at 11:38:19AM -0500, Todd Zullinger wrote:

> With perl-Getopt-Long >= 2.55, a warning is issued for options which are
> specified more than once.  In addition to causing users to see warnings,
> this results in test failures which compare the output.  An example,
> from t9001-send-email.37:

This made me wonder if the warnings are new, or if the duplicated
auto-negated options are new. I.e., were the manual "--no-foo" option
specs doing something useful in the older versions (in which case we'd
need to do something more complicated)?

But I think the answer is no.  We've explicitly marked these with "!" to
indicate that they're negatable. And certainly running with Getopt::Long
2.52 (from perl 5.36, which is the current in Debian unstable) seems to
support them.

It does make me wonder why some boolean options are not marked as
negatable (even if just to countermand an earlier option), but that is
outside the scope of your patch.

> I've run this through the full test suite.  I also compared the output of
> --help to ensure it only differs in the removal of the "Duplicate
> specification" warnings.  I _think_ that's a good sign that no other changes
> will result.  But I would be grateful to anyone who can confirm or reject that
> theory.

I guess you meant "-h", not "--help", since the latter will just show
the manpage. But isn't "-h" just dumping a static usage message we
wrote, and not auto-generated by the code?

The changes look good to me (even after double-checking Junio's question
that they are all appropriately matched with their "positive" sides).
This one is curious:

> -		    "cc-cover|cc-cover!" => \$cover_cc,

It was an alternate name for itself? I think somebody just misunderstood
how the API was supposed to work. The "!" would applies to all names, if
I understand correctly, so this really is doing nothing beyond just
"cc-cover!", which is what your patch switches it to.

-Peff