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

[Oswald Buddenhagen <oswald.buddenhagen@gmx.de>]

On Mon, Aug 07, 2023 at 11:55:29AM -0700, Junio C Hamano wrote:
>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.

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

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

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