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