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

From: Todd Zullinger <tmz@pobox.com>
Date: Nov 14, 2023, 20:59 UTC
Message-ID: <ZVPfvjoXyGVlKqvr@pobox.com>
In-Reply-To: <20231114200009.GD2092538@coredump.intra.peff.net>

Jeff King wrote:

Show 10 quoted lines

> On Tue, Nov 14, 2023 at 11:38:19AM -0500, Todd Zullinger wrote:
>> 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?

Yes to both. This is why I shouldn't submit patches within a few hours of waking up.

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

Indeed. I need to go through them each to test that the results match before and after. With the fallback to passing options to format-patch, testing outside of a git repo makes this rather convenient. If I've dropped an option it will result in the "Cannot run git format-patch from outside a repository" error. That's a good start to ensure the changes don't cause any regressions.

I did notice that I mistakenly dropped --[no-]signed-off-cc. I need to keep:

    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,

as is.

Show 8 quoted lines

> 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.

I wondered about those as well. Perhaps this is needed in some older version of Getopt::Long? I'll try to look through the history of the module to see if that's the case.

Since this isn't anything new with 2.43, it doesn't need to be fixed with much urgency.

Thanks both,

-- 
Todd