Re: [PATCH v3] send-email: prompt-dependent exit codes

From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Date: Aug 8, 2023, 10:55 UTC
Message-ID: <ZNIfDXJBqEVcHh+D@ugly>
In-Reply-To: <xmqqbkfifzry.fsf@gitster.g>

On Mon, Aug 07, 2023 at 11:55:29AM -0700, Junio C Hamano wrote:

Show 8 quoted lines

>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>
>> From the perspective of a scripted caller, failure to send (some) mails
>> is an error even if it was interactively requested, so it should be
>> indicated by the exit code.
>
>I am not sure if unconditional change of exit code this late in the
>game.

> When was the interactive "no, do not send this one" feature 
> introduced? 
>

c1f2aa45b7 (add --confirm option and configuration setting, 2009-03-02), and extended by 5c80afed02 (ask what to do with an invalid email address, 2012-11-26), so basically it's been there forever.

>I wonder if this should be hidden behind an opt-in command line option 
>and possibly a configuration variable that defaults to "no".
>

i wondered, too, but i think it isn't really worth it. interactive users won't really care (see below), and most scripted users presumably simply suppressed the condition by passing --confirm=never and/or appropriate --suppress* options. others didn't notice (possibly because of their config options), or ignored the problem, and therefore have a good chance of being broken. some were unhappy about it, but didn't bother reporting/patching it, which always constitutes the vast majority of affected users. this still leaves us with a hypothetical small set of wrapper scripts that _really_ want to remain ignorant of messages being skipped. i think it's acceptable to expect them to adjust to the (from their POV) false positives, as it should be mostly harmless, and have the effect of getting some scripts fixed.

Show 8 quoted lines

>> To make it somewhat specific, the exit code is 10 when only some 
>> mails
>> were skipped, and 11 if the user quit on the first prompt.
>
>If 10 and 11 were *not* taken out of thin air, but there is a
>precedent to use these two values in e-mail related programs, please
>share.  It may give us a good justification.
>

the numbers are indeed pulled out of thin air. the offset is chosen such that it's sufficiently far off normally expected exit codes.

Show 5 quoted lines

>With or without other people's precedents, and with or without
>making it conditional, the new behaviour must be documented, if the
>command has already a documentation (and it seems that there exists
>the Documentation/git-send-email.txt file).
>

ack, will add an EXIT STATUS section.

>It may be preferrable
>to protect the new feature with a test or two added to t9001 but it
>obviously depends on how hard you find testing interactive stuff is.
>

shouldn't be too hard; test_{,no_}confirm show how to do it.

Show 7 quoted lines

>> For interactive calls from the command line, interactive cancellation is
>> arguably not really an error, but there the exit code will be more or
>> less ignored anyway.
>
>Not necessarily.  Some people prefer to see it and show it in their
>command line prompt.
>

hence "mostly". but users generally know what they did just a few secs ago, so likely won't be bothered by the special exit code.

>Thanks.  Will queue but expect at least some documentation updates.
>

do you want a followup, or a v4 to replace the commit?

regards