{"thread":{"id":"59658","subject":"[PATCH v2] send-email: prompt-dependent exit codes","startedAt":"2023-04-26T06:16:11Z","lastAt":"2023-08-30T18:29:33Z","messageCount":18,"participants":["Oswald Buddenhagen","Junio C Hamano","Felipe Contreras"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"476089","messageId":"20230426061606.1495646-1-oswald.buddenhagen@gmx.de","threadId":"59658","inReplyTo":null,"subject":"[PATCH v2] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-04-26T06:16:06Z","receivedAt":"2023-04-26T06:16:11Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"From the perspective of the caller, failure to send (some) mails is an\nerror even if it was interactively requested, so it should be indicated\nby the exit code.\n\nTo make it somewhat specific, the exit code is 10 when only some mails\nwere skipped, and 11 if the user quit on the first prompt.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\nv2:\n- fix do_quit() not resetting $sent_all\n---\n git-send-email.perl | 27 +++++++++++++++++++++++----\n 1 file changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 07f2a0cbea..7587cd2d20 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -254,6 +254,19 @@ sub system_or_die {\n \tdie $msg if $msg;\n }\n \n+my $sent_any = 0;\n+my $sent_all = 1;\n+\n+sub do_exit {\n+\texit($sent_any ? $sent_all ? 0 : 10 : 11);\n+}\n+\n+sub do_quit {\n+\tcleanup_compose_files();\n+\t$sent_all = 0;\n+\tdo_exit();\n+}\n+\n sub do_edit {\n \tif (!defined($editor)) {\n \t\t$editor = Git::command_oneline('var', 'GIT_EDITOR');\n@@ -1172,8 +1185,7 @@ sub validate_address {\n \t\tif (/^d/i) {\n \t\t\treturn undef;\n \t\t} elsif (/^q/i) {\n-\t\t\tcleanup_compose_files();\n-\t\t\texit(0);\n+\t\t\tdo_quit();\n \t\t}\n \t\t$address = ask(\"$to_whom \",\n \t\t\tdefault => \"\",\n@@ -1593,8 +1605,7 @@ sub send_message {\n \t\t} elsif (/^e/i) {\n \t\t\treturn -1;\n \t\t} elsif (/^q/i) {\n-\t\t\tcleanup_compose_files();\n-\t\t\texit(0);\n+\t\t\tdo_quit();\n \t\t} elsif (/^a/i) {\n \t\t\t$confirm = 'never';\n \t\t}\n@@ -1968,6 +1979,12 @@ sub process_file {\n \t\treturn 0;\n \t}\n \n+\tif ($message_was_sent) {\n+\t\t$sent_any = 1;\n+\t} else {\n+\t\t$sent_all = 0;\n+\t}\n+\n \t# set up for the next message\n \tif ($thread) {\n \t\tif ($message_was_sent &&\n@@ -2187,3 +2204,5 @@ sub body_or_subject_has_nonascii {\n \t}\n \treturn 0;\n }\n+\n+do_exit();\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"476210","messageId":"xmqqttx1l3zp.fsf@gitster.g","threadId":"59658","inReplyTo":"20230426061606.1495646-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v2] send-email: prompt-dependent exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-27T15:49:30Z","receivedAt":"2023-04-27T15:49:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> From the perspective of the caller, failure to send (some) mails is an\n> error even if it was interactively requested, so it should be indicated\n> by the exit code.\n>\n> To make it somewhat specific, the exit code is 10 when only some mails\n> were skipped, and 11 if the user quit on the first prompt.\n>\n> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n>\n> ---\n> v2:\n> - fix do_quit() not resetting $sent_all\n> ---\n>  git-send-email.perl | 27 +++++++++++++++++++++++----\n>  1 file changed, 23 insertions(+), 4 deletions(-)\n\nAdministrivia:\n\n * Marking the patch as \"v2\" is very good to signal that this is an\n   improved version of a previous effort.  It also is very good that\n   the difference from \"v1\" is summarized below the three-dash line.\n\n * When sending such a new iteration, it helps readers and reviewers\n   to make it a reply to the previous round.  You seem to be using\n   \"git send-email\" and giving the option\n\n   --in-reply-to=20230323162234.995435-1-oswald.buddenhagen@gmx.de\n\n   would have made this message a reply to the previous one.  Not\n   everybody will remember your previous attempt.\n\n * It gives the new iteration a better chance to be reviewed if you\n   CC'ed the right people, including the ones who gave reviews on\n   the previous round.\n\nAs to the motivation described in the proposed log message, I am not\nconvinced.  The end-user knows the best, they told the program to\nstop, and the program stopped as it was told.  I tend to think that\nit should not be treated as an error.  But if somebody is driving\n\"git send-email\" from a wrapper that wants to know what happened, I\ncan understand that \"did we send everything we were told to send\" is\n*one* of the things such a wrapper may want to know.  And my earlier\n\"I am not convinced\" is definitely not \"I am convinced that what\nthis patch wants to do is wrong\".\n\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 07f2a0cbea..7587cd2d20 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -254,6 +254,19 @@ sub system_or_die {\n>  \tdie $msg if $msg;\n>  }\n>  \n> +my $sent_any = 0;\n> +my $sent_all = 1;\n> +\n> +sub do_exit {\n> +\texit($sent_any ? $sent_all ? 0 : 10 : 11);\n> +}\n\nWithout knowing where \"v1\" was (see Administrivia above) and seeing\nthe use of above two variables, I found them a bit unnatural and was\ngoing to suggest to have the total number of messages and the number\nof messages that were actually sent, but it turns out that it was\nthe exact same reaction as Phillip had for \"v1\" iteration.\n\nIt would work either way until we start caring how many we actually\nhave sent, but keeping tallies may be more future-proof.\n\nIn any case, before sending anything, we initialize ourselves to the\n\"we haven't sent anything\" and \"we have sent everything we have been\nasked to send so far\" state.  The idea to flip the former flag to true\nonce we sent a single message while flipping the latter flag to false\nonce we failed to send a single message to maintain these two flags is\nsimple and effective.\n\n> +sub do_quit {\n> +\tcleanup_compose_files();\n> +\t$sent_all = 0;\n> +\tdo_exit();\n> +}\n> +\n>  sub do_edit {\n>  \tif (!defined($editor)) {\n>  \t\t$editor = Git::command_oneline('var', 'GIT_EDITOR');\n> @@ -1172,8 +1185,7 @@ sub validate_address {\n>  \t\tif (/^d/i) {\n>  \t\t\treturn undef;\n>  \t\t} elsif (/^q/i) {\n> -\t\t\tcleanup_compose_files();\n> -\t\t\texit(0);\n> +\t\t\tdo_quit();\n>  \t\t}\n>  \t\t$address = ask(\"$to_whom \",\n>  \t\t\tdefault => \"\",\n> @@ -1593,8 +1605,7 @@ sub send_message {\n>  \t\t} elsif (/^e/i) {\n>  \t\t\treturn -1;\n>  \t\t} elsif (/^q/i) {\n> -\t\t\tcleanup_compose_files();\n> -\t\t\texit(0);\n> +\t\t\tdo_quit();\n>  \t\t} elsif (/^a/i) {\n>  \t\t\t$confirm = 'never';\n>  \t\t}\n\nOK, the above two covers the case where the end-user interactively\ntells the command to stop before sending the message in question,\nand by definition in such a case, at least one message (i.e. the one\nthat triggered the interactive session) was not sent, so dropping\nthe \"all\" flag makes sense.\n\n> @@ -1968,6 +1979,12 @@ sub process_file {\n>  \t\treturn 0;\n>  \t}\n>  \n> +\tif ($message_was_sent) {\n> +\t\t$sent_any = 1;\n> +\t} else {\n> +\t\t$sent_all = 0;\n> +\t}\n\nAnd after processing a single file, we adjust the \"any\" and \"all\"\ncounter.\n\nThe only semi-tricky part of the whole set-up, I presume, is that\nthe \"I've sent everything I was asked to send so far\" flag needs to\nbe initialized to true, and the \"I have sent anything yet\" flag to\nfalse.  When the number of messages we were given is 0, after\niterating over the loop 0 times and never calling process_file,\ndo_exit will see that we haven't sent any and we have sent all that\nwere asked to be sent.  Is that an error?  According to the\nhard-to-read nested ternary in do_exit, it is:\n\n\texit($sent_any ? $sent_all ? 0 : 10 : 11);\n\nbecause $sent_any would be 0.  I wonder if it can be remedied by a\nsimple reordering to check $sent_all first?  I.e. something like (as\nI do not think anybody can read nested ternary and keep their\nsanity) this?\n\n\tif ($sent_all) {\n\t\texit(0);\n\t} elsif ($sent_any) {\n\t\texit(11);\n\t} else {\n\t\texit(10);\n\t}\n\n>  \t# set up for the next message\n>  \tif ($thread) {\n>  \t\tif ($message_was_sent &&\n> @@ -2187,3 +2204,5 @@ sub body_or_subject_has_nonascii {\n>  \t}\n>  \treturn 0;\n>  }\n> +\n> +do_exit();\n\nThanks.\n"},{"id":"476452","messageId":"64515ec34592a_1ba2d29466@chronos.notmuch","threadId":"59658","inReplyTo":"xmqqttx1l3zp.fsf@gitster.g","subject":"Re: [PATCH v2] send-email: prompt-dependent exit codes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-02T19:04:35Z","receivedAt":"2023-05-02T19:04:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n\n> Administrivia:\n> \n>  * Marking the patch as \"v2\" is very good to signal that this is an\n>    improved version of a previous effort.  It also is very good that\n>    the difference from \"v1\" is summarized below the three-dash line.\n> \n>  * When sending such a new iteration, it helps readers and reviewers\n>    to make it a reply to the previous round.  You seem to be using\n>    \"git send-email\" and giving the option\n> \n>    --in-reply-to=20230323162234.995435-1-oswald.buddenhagen@gmx.de\n> \n>    would have made this message a reply to the previous one.  Not\n>    everybody will remember your previous attempt.\n\nYou can also use `git send-series` which does that for you, and also\nsends the `git range-diff` automatically.\n\nhttps://github.com/felipec/git-send-series\n\n-- \nFelipe Contreras\n"},{"id":"480229","messageId":"20230807165850.2335067-1-oswald.buddenhagen@gmx.de","threadId":"59658","inReplyTo":"xmqqttx1l3zp.fsf@gitster.g","subject":"[PATCH v3] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-07T16:58:50Z","receivedAt":"2023-08-07T16:59:07Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"From the perspective of a scripted caller, failure to send (some) mails\nis an error even if it was interactively requested, so it should be\nindicated by the exit code.\n\nTo make it somewhat specific, the exit code is 10 when only some mails\nwere skipped, and 11 if the user quit on the first prompt.\n\nFor interactive calls from the command line, interactive cancellation is\narguably not really an error, but there the exit code will be more or\nless ignored anyway.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\nv3:\n- use a tally instead of flags after all, as my seemingly simple idea\n  apparently requires lots of thinking to grasp fully\n- correct exit code when zero messages are to be sent. this cannot\n  actually happen, as it induces an exit via usage() earlier on.\n- unfold nested ternary to save junio's sanity (who proved his point by\n  unfolding it slightly incorrectly)\n- expand commit message\nv2:\n- fix do_quit() not resetting $sent_all\n\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Phillip Wood <phillip.wood123@gmail.com>\n---\n git-send-email.perl | 32 ++++++++++++++++++++++++++++----\n 1 file changed, 28 insertions(+), 4 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex affbb88509..cd4db84b7f 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -256,6 +256,26 @@ sub system_or_die {\n \tdie $msg if $msg;\n }\n \n+my $sent_files = 0;\n+\n+sub do_exit {\n+\tif ($sent_files == @files) {\n+\t\t# All specified messages were sent\n+\t\texit(0);\n+\t} elsif ($sent_files) {\n+\t\t# At least some messages were sent\n+\t\texit(10);\n+\t} else {\n+\t\t# User cancelled on first message already\n+\t\texit(11);\n+\t}\n+}\n+\n+sub do_quit {\n+\tcleanup_compose_files();\n+\tdo_exit();\n+}\n+\n sub do_edit {\n \tif (!defined($editor)) {\n \t\t$editor = Git::command_oneline('var', 'GIT_EDITOR');\n@@ -1195,8 +1215,7 @@ sub validate_address {\n \t\tif (/^d/i) {\n \t\t\treturn undef;\n \t\t} elsif (/^q/i) {\n-\t\t\tcleanup_compose_files();\n-\t\t\texit(0);\n+\t\t\tdo_quit();\n \t\t}\n \t\t$address = ask(\"$to_whom \",\n \t\t\tdefault => \"\",\n@@ -1619,8 +1638,7 @@ sub send_message {\n \t\t} elsif (/^e/i) {\n \t\t\treturn -1;\n \t\t} elsif (/^q/i) {\n-\t\t\tcleanup_compose_files();\n-\t\t\texit(0);\n+\t\t\tdo_quit();\n \t\t} elsif (/^a/i) {\n \t\t\t$confirm = 'never';\n \t\t}\n@@ -2001,6 +2019,10 @@ sub process_file {\n \t\treturn 0;\n \t}\n \n+\tif ($message_was_sent) {\n+\t\t$sent_files++;\n+\t}\n+\n \t# set up for the next message\n \tif ($thread) {\n \t\tif ($message_was_sent &&\n@@ -2278,3 +2300,5 @@ sub body_or_subject_has_nonascii {\n \t}\n \treturn 0;\n }\n+\n+do_exit();\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480249","messageId":"xmqqbkfifzry.fsf@gitster.g","threadId":"59658","inReplyTo":"20230807165850.2335067-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v3] send-email: prompt-dependent exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-07T18:55:29Z","receivedAt":"2023-08-07T18:55:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> From the perspective of a scripted caller, failure to send (some) mails\n> is an error even if it was interactively requested, so it should be\n> indicated by the exit code.\n\nI am not sure if unconditional change of exit code this late in the\ngame.  When was the interactive \"no, do not send this one\" feature\nintroduced?  The end-users (not necessarily all of them, of course)\nhave been happily using the command and have appreciated not having\nto see non-zero exit code after skipping some messages.  I wonder if\nthis should be hidden behind an opt-in command line option and possibly\na configuration variable that defaults to \"no\".\n\n> To make it somewhat specific, the exit code is 10 when only some mails\n> were skipped, and 11 if the user quit on the first prompt.\n\nIf 10 and 11 were *not* taken out of thin air, but there is a\nprecedent to use these two values in e-mail related programs, please\nshare.  It may give us a good justification.\n\nWith or without other people's precedents, and with or without\nmaking it conditional, the new behaviour must be documented, if the\ncommand has already a documentation (and it seems that there exists\nthe Documentation/git-send-email.txt file).  It may be preferrable\nto protect the new feature with a test or two added to t9001 but it\nobviously depends on how hard you find testing interactive stuff is.\n\n> For interactive calls from the command line, interactive cancellation is\n> arguably not really an error, but there the exit code will be more or\n> less ignored anyway.\n\nNot necessarily.  Some people prefer to see it and show it in their\ncommand line prompt.\n\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index affbb88509..cd4db84b7f 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -256,6 +256,26 @@ sub system_or_die {\n>  \tdie $msg if $msg;\n>  }\n>  \n> +my $sent_files = 0;\n> +\n> +sub do_exit {\n> +\tif ($sent_files == @files) {\n> +\t\t# All specified messages were sent\n> +\t\texit(0);\n> +\t} elsif ($sent_files) {\n> +\t\t# At least some messages were sent\n> +\t\texit(10);\n> +\t} else {\n> +\t\t# User cancelled on first message already\n> +\t\texit(11);\n> +\t}\n> +}\n\nOK.  As log as we won't start doing \n\n\twhile (loop) {\n\t\t$file = shift @files;\n\t\tsend $file;\n\t}\n\nthis will keep working fine, and the logic is very clear to see.\n\nThanks.  Will queue but expect at least some documentation updates.\n\n\n"},{"id":"480279","messageId":"ZNIfDXJBqEVcHh+D@ugly","threadId":"59658","inReplyTo":"xmqqbkfifzry.fsf@gitster.g","subject":"Re: [PATCH v3] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-08T10:55:09Z","receivedAt":"2023-08-08T15:58:47Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Mon, Aug 07, 2023 at 11:55:29AM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> From the perspective of a scripted caller, failure to send (some) mails\n>> is an error even if it was interactively requested, so it should be\n>> indicated by the exit code.\n>\n>I am not sure if unconditional change of exit code this late in the\n>game.\n\n> When was the interactive \"no, do not send this one\" feature \n> introduced? \n>\nc1f2aa45b7 (add --confirm option and configuration setting, 2009-03-02), \nand extended by 5c80afed02 (ask what to do with an invalid email \naddress, 2012-11-26), so basically it's been there forever.\n\n>I wonder if this should be hidden behind an opt-in command line option \n>and possibly a configuration variable that defaults to \"no\".\n>\ni wondered, too, but i think it isn't really worth it.\ninteractive users won't really care (see below), and most scripted users \npresumably simply suppressed the condition by passing --confirm=never \nand/or appropriate --suppress* options. others didn't notice (possibly \nbecause of their config options), or ignored the problem, and therefore \nhave a good chance of being broken. some were unhappy about it, but \ndidn't bother reporting/patching it, which always constitutes the vast \nmajority of affected users. this still leaves us with a hypothetical \nsmall set of wrapper scripts that _really_ want to remain ignorant of \nmessages being skipped. i think it's acceptable to expect them to adjust \nto the (from their POV) false positives, as it should be mostly \nharmless, and have the effect of getting some scripts fixed.\n\n>> To make it somewhat specific, the exit code is 10 when only some \n>> mails\n>> were skipped, and 11 if the user quit on the first prompt.\n>\n>If 10 and 11 were *not* taken out of thin air, but there is a\n>precedent to use these two values in e-mail related programs, please\n>share.  It may give us a good justification.\n>\nthe numbers are indeed pulled out of thin air. the offset is chosen such \nthat it's sufficiently far off normally expected exit codes.\n\n>With or without other people's precedents, and with or without\n>making it conditional, the new behaviour must be documented, if the\n>command has already a documentation (and it seems that there exists\n>the Documentation/git-send-email.txt file).\n>\nack, will add an EXIT STATUS section.\n\n>It may be preferrable\n>to protect the new feature with a test or two added to t9001 but it\n>obviously depends on how hard you find testing interactive stuff is.\n>\nshouldn't be too hard; test_{,no_}confirm show how to do it.\n\n>> For interactive calls from the command line, interactive cancellation is\n>> arguably not really an error, but there the exit code will be more or\n>> less ignored anyway.\n>\n>Not necessarily.  Some people prefer to see it and show it in their\n>command line prompt.\n>\nhence \"mostly\".\nbut users generally know what they did just a few secs ago, so likely \nwon't be bothered by the special exit code.\n\n>Thanks.  Will queue but expect at least some documentation updates.\n>\ndo you want a followup, or a v4 to replace the commit?\n\nregards\n"},{"id":"480294","messageId":"xmqqmsz1a547.fsf@gitster.g","threadId":"59658","inReplyTo":"ZNIfDXJBqEVcHh+D@ugly","subject":"Re: [PATCH v3] send-email: prompt-dependent exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-08T16:08:56Z","receivedAt":"2023-08-08T17:23:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> i think it's acceptable to expect them to adjust to the (from\n> their POV) false positives,...\n\nYou are not in the position to unilaterally declare \"it isn't really\nworth it\" or \"it's acceptable to expect\", though.  We know at least\none person want the program to signal failure when interactively\naborted.  We know that all other current users did not complain that\nthe program did not signal failure when they interactively aborted,\nthey may be they did not care enough, or they may be they are happy.\nYou do not know the population and neither do I, but that is not the\npoint.  This behaviour change is, even among Git developers, not a\nbugfix but is a new feature that not everybody would want to be\nsubjected to.  It always is the safest to make such a backward\nincompatible change a strictly opt-in feature.\n\n>>Thanks.  Will queue but expect at least some documentation updates.\n>>\n> do you want a followup, or a v4 to replace the commit?\n\nReplace it.  We do not want to do \"oops that was lacking, here is an\nincremental update\" for anything that is not in 'next'.\n\nThanks.\n"},{"id":"480313","messageId":"ZNKTXPVK1p1oHhtE@ugly","threadId":"59658","inReplyTo":"xmqqmsz1a547.fsf@gitster.g","subject":"Re: [PATCH v3] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-08T19:11:24Z","receivedAt":"2023-08-08T19:41:02Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Tue, Aug 08, 2023 at 09:08:56AM -0700, Junio C Hamano wrote:\n>This behaviour change is, even among Git developers, not a bugfix but \n>is a new feature that not everybody would want to be subjected to.\n>\nthat's debatable, depending on one's expectations. at the very least \nit's an enabler for bugfixes.\n\n>It always is the safest to make such a backward incompatible change a \n>strictly opt-in feature.\n>\nwell, yes. the question is whether we expect non-trivial problems \narising from imposing it on unsuspecting users. and given that there is \nan (opportunity) cost of having it opt-in only, there is a trade-off. my \ngut feeling is that having it always-on is probably better.\n\nregards\n"},{"id":"480373","messageId":"20230809171531.2564739-1-oswald.buddenhagen@gmx.de","threadId":"59658","inReplyTo":"xmqqmsz1a547.fsf@gitster.g","subject":"[PATCH v4] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-09T17:15:31Z","receivedAt":"2023-08-09T17:15:45Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"From the perspective of a scripted caller, failure to send (some) mails\nis an error even if it was interactively requested, so it should be\nindicated by a non-zero exit code.\n\nThis is technically a backwards-incompatible behavior change, and\ntherefore would be safer to make opt-in. However, the damage resulting\nfrom reporting the situation too eagerly is expected to be trivial,\nwhile the damage resulting from callers failing to opt-in (which is\nobviously the status quo, and is likely to persist in most cases) is\npotentially at least somewhat serious.\n\nFor interactive calls from the command line, interactive cancellation is\narguably not really an error, but a human user will be able to interpret\nthe somewhat unusual exit code in light of their immediately preceding\ninteractions just fine.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\nv4:\n- add tests (which also cover the partial confirmation behavior in the\n  first place)\n- add docu\n- rework commit message again\nv3:\n- use a tally instead of flags after all, as my seemingly simple idea\n  apparently requires lots of thinking to grasp fully\n- correct exit code when zero messages are to be sent. this cannot\n  actually happen, as it induces an exit via usage() earlier on.\n- unfold nested ternary to save junio's sanity (who proved his point by\n  unfolding it slightly incorrectly)\n- expand commit message\nv2:\n- fix do_quit() not resetting $sent_all\n\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Phillip Wood <phillip.wood123@gmail.com>\n---\n Documentation/git-send-email.txt |  9 ++++++++\n git-send-email.perl              | 32 ++++++++++++++++++++++++----\n t/t9001-send-email.sh            | 36 ++++++++++++++++++++++++++++++++\n 3 files changed, 73 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 492a82323d..d6ce3a0bd9 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -480,6 +480,15 @@ include::includes/cmd-config-section-all.txt[]\n \n include::config/sendemail.txt[]\n \n+\n+EXIT STATUS\n+-----------\n+\n+Zero is returned when all specified patches were sent, while one is returned\n+when an error occurs. 10 is returned if the user interactively skips sending\n+some patches, and 11 if they skip all patches.\n+\n+\n EXAMPLES\n --------\n Use gmail as the smtp server\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex affbb88509..cd4db84b7f 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -256,6 +256,26 @@ sub system_or_die {\n \tdie $msg if $msg;\n }\n \n+my $sent_files = 0;\n+\n+sub do_exit {\n+\tif ($sent_files == @files) {\n+\t\t# All specified messages were sent\n+\t\texit(0);\n+\t} elsif ($sent_files) {\n+\t\t# At least some messages were sent\n+\t\texit(10);\n+\t} else {\n+\t\t# User cancelled on first message already\n+\t\texit(11);\n+\t}\n+}\n+\n+sub do_quit {\n+\tcleanup_compose_files();\n+\tdo_exit();\n+}\n+\n sub do_edit {\n \tif (!defined($editor)) {\n \t\t$editor = Git::command_oneline('var', 'GIT_EDITOR');\n@@ -1195,8 +1215,7 @@ sub validate_address {\n \t\tif (/^d/i) {\n \t\t\treturn undef;\n \t\t} elsif (/^q/i) {\n-\t\t\tcleanup_compose_files();\n-\t\t\texit(0);\n+\t\t\tdo_quit();\n \t\t}\n \t\t$address = ask(\"$to_whom \",\n \t\t\tdefault => \"\",\n@@ -1619,8 +1638,7 @@ sub send_message {\n \t\t} elsif (/^e/i) {\n \t\t\treturn -1;\n \t\t} elsif (/^q/i) {\n-\t\t\tcleanup_compose_files();\n-\t\t\texit(0);\n+\t\t\tdo_quit();\n \t\t} elsif (/^a/i) {\n \t\t\t$confirm = 'never';\n \t\t}\n@@ -2001,6 +2019,10 @@ sub process_file {\n \t\treturn 0;\n \t}\n \n+\tif ($message_was_sent) {\n+\t\t$sent_files++;\n+\t}\n+\n \t# set up for the next message\n \tif ($thread) {\n \t\tif ($message_was_sent &&\n@@ -2278,3 +2300,5 @@ sub body_or_subject_has_nonascii {\n \t}\n \treturn 0;\n }\n+\n+do_exit();\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 48bf0af2ee..dce0b1c498 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1187,6 +1187,42 @@ test_expect_success $PREREQ 'confirm does not loop forever' '\n \t\t\t$patches\n '\n \n+test_confirm_many () {\n+\tclean_fake_sendmail\n+\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\tgit send-email \\\n+\t\t--confirm=auto \\\n+\t\t--from=\"Example <nobody@example.com>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t-2 <replies\n+\techo $? >exit_sts\n+\ttest_cmp expected_sts exit_sts || return 1\n+\tls commandline* 2>/dev/null | wc -l > num_mails\n+\ttest_cmp expected_num num_mails || return 1\n+}\n+\n+test_expect_success $PREREQ 'interactively skip none' '\n+\t(echo y && echo y) > replies &&\n+\techo 0 > expected_sts &&\n+\techo 2 > expected_num &&\n+\ttest_confirm_many\n+'\n+\n+test_expect_success $PREREQ 'interactively skip some' '\n+\t(echo n && echo y) > replies &&\n+\techo 10 > expected_sts &&\n+\techo 1 > expected_num &&\n+\ttest_confirm_many\n+'\n+\n+test_expect_success $PREREQ 'interactively skip all' '\n+\t(echo n && echo n) > replies &&\n+\techo 11 > expected_sts &&\n+\techo 0 > expected_num &&\n+\ttest_confirm_many\n+'\n+\n test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '\n \tclean_fake_sendmail &&\n \trm -fr outdir &&\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480389","messageId":"xmqqo7jgm3hd.fsf@gitster.g","threadId":"59658","inReplyTo":"20230809171531.2564739-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v4] send-email: prompt-dependent exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-09T19:15:42Z","receivedAt":"2023-08-09T19:17:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> From the perspective of a scripted caller, failure to send (some) mails\n> is an error even if it was interactively requested, so it should be\n> indicated by a non-zero exit code.\n\nI would agree that there should be a way to ask the command to\nindicate if some messages were not sent due to end-user request, but\nI have to say \"From the perspective of a scripted caller\" is a gross\nover generalization that I would not want to see in a commit log\nmessage of the project I run.  Not all our scripters do not write\nthe scripts the way you do.\n\nIt should not be hard to make this opt-in, and I still think it\nshould be opt-in.\n\nThanks.\n"},{"id":"480429","messageId":"ZNS1VyGgtk9JmMe0@ugly","threadId":"59658","inReplyTo":"xmqqo7jgm3hd.fsf@gitster.g","subject":"Re: [PATCH v4] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-10T10:00:55Z","receivedAt":"2023-08-10T10:03:32Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Wed, Aug 09, 2023 at 12:15:42PM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> From the perspective of a scripted caller, failure to send (some) mails\n>> is an error even if it was interactively requested, so it should be\n>> indicated by a non-zero exit code.\n>\n>I would agree that there should be a way to ask the command to\n>indicate if some messages were not sent due to end-user request, but\n>I have to say \"From the perspective of a scripted caller\" is a gross\n>over generalization that I would not want to see in a commit log\n>message of the project I run.\n>\nthis gross error can be fixed by adding \"likely\" to the sentence.\n\n>It should not be hard to make this opt-in,\n>\nthat doesn't matter.\n\n>and I still think it should be opt-in.\n>\nand i still think this would significantly reduce the value of the \nchange.\n\nthe very idea of having to explicitly request that the obviously right \nthing is done is intuitively silly, and shouldn't be seriously \nentertained unless changing existing behavior can plausibly lead to \nserious adverse consequences. the minor nuisance of having to adjust \nwrappers to explicitly accept the most likely unexpected case does not \nqualify as such.\n\nfwiw, other users who noticed this problem most probably preempted it by \nadding appropriate --confirm=* and --suppress-* options. but this \ndoesn't fit my use case of a \"light\" wrapper.\n\nregards\n"},{"id":"480465","messageId":"xmqq5y5msmc0.fsf@gitster.g","threadId":"59658","inReplyTo":"ZNS1VyGgtk9JmMe0@ugly","subject":"Re: [PATCH v4] send-email: prompt-dependent exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-10T19:56:31Z","receivedAt":"2023-08-10T19:56:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> fwiw, other users who noticed this problem most probably preempted it\n> by adding appropriate --confirm=* and --suppress-* options. but this\n> doesn't fit my use case of a \"light\" wrapper.\n\nI have no problem with giving the command an option to allow it to\nreport if one or more messages were held back via its exit status.\n\nOthers may even want to know which ones among many messages weren't\nsent, not just \"there were some that weren't sent\", to make it\neasier to identify which ones to be resent after touching them up.\n\nIn any case, I consider such changes (including the \"were there any\nthat weren't sent\" bit) optional new features.\n\nThanks.\n\n"},{"id":"480531","messageId":"ZNYljcYTKHZ6hAyR@ugly","threadId":"59658","inReplyTo":"xmqq5y5msmc0.fsf@gitster.g","subject":"Re: [PATCH v4] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-11T12:11:57Z","receivedAt":"2023-08-11T12:12:05Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Thu, Aug 10, 2023 at 12:56:31PM -0700, Junio C Hamano wrote:\n>I have no problem with giving the command an option to allow it to\n>report if one or more messages were held back via its exit status.\n>\nthat doesn't give credit to the observation that there is an opportunity \ncost to not doing it automatically. it is much easier to imagine that a \nscripting user simply didn't consider the possibility of an interactive \ncancellation than that they really meant to ignore it. therefore, the \nstatus quo is very likely more harmful than the fallout of starting to \nmore aggressively report the situation.\n\n>Others may even want to know which ones among many messages weren't\n>sent, not just \"there were some that weren't sent\", to make it\n>easier to identify which ones to be resent after touching them up.\n>\nyeah, i've considered that as well.\ni think that on the exit status side my solution is adequate - it \nreports in a sufficiently distinctive way that something didn't go quite \nas probably expected.\n\ngoing further, i'm considering producing a mapping from sha1s to \nmessage-ids (which i'd persist for automatic chaining of re-rolls). this \nwould provide the fine-grained reporting as a side effect.\n\nregards\n"},{"id":"480845","messageId":"20230821170720.577835-1-oswald.buddenhagen@gmx.de","threadId":"59658","inReplyTo":"xmqq5y5msmc0.fsf@gitster.g","subject":"[PATCH v5] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-21T17:07:20Z","receivedAt":"2023-08-21T17:07:28Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"It seems very likely that most scripted callers would want to know when\n(some) mails were interactively requested to be not sent, so indicate\nthis situation with a non-zero exit code. We use 10/11, because these\nseem sufficiently distinct from \"regular\" error codes one would expect.\n\nThis is technically a backwards-incompatible behavior change, and\ntherefore would be safer to make opt-in. However, it is much easier to\nimagine that a scripting user simply didn't consider the possibility of\nan interactive cancellation, than that they really meant to ignore it.\nAlso, the damage resulting from reporting the situation too eagerly is\nexpected to be trivial, while the damage resulting from callers failing\nto opt-in (which is obviously the status quo, and is likely to persist\nin most cases) is potentially at least somewhat serious. This means that\nmaking it opt-in has an opportunity cost, and I think the trade-off\nfavors making the new behavior unconditional.\n\nFor interactive calls from the command line, interactive cancellation is\narguably not unexpected, but a human user will be able to interpret the\nsomewhat unusual exit code in light of their immediately preceding\ninteractions just fine.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\nProposed content for RelNotes:\n\n * \"git send-email\" now reports interactive cancellation via a distinct\n   non-zero exit status. Callers which do not consider this situation an\n   error need to be adjusted.\n\n---\nv5:\n- fix whitespace in tests' redirections\n- tweak commit message some more\n- tweak manual (notably, it now says \"1\" instead of \"one\", which is\n  linguistically incorrect, but imo less confusing)\n- fix inaccuracy in a comment in do_exit()\nv4:\n- add tests (which also cover the partial confirmation behavior in the\n  first place)\n- add docu\n- rework commit message again\nv3:\n- use a tally instead of flags after all, as my seemingly simple idea\n  apparently requires lots of thinking to grasp fully\n- correct exit code when zero messages are to be sent. this cannot\n  actually happen, as it induces an exit via usage() earlier on.\n- unfold nested ternary to save junio's sanity (who proved his point by\n  unfolding it slightly incorrectly)\n- expand commit message\nv2:\n- fix do_quit() not resetting $sent_all\n\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Phillip Wood <phillip.wood123@gmail.com>\n---\n Documentation/git-send-email.txt |  9 ++++++++\n git-send-email.perl              | 32 ++++++++++++++++++++++++----\n t/t9001-send-email.sh            | 36 ++++++++++++++++++++++++++++++++\n 3 files changed, 73 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 492a82323d..766c190ef0 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -480,6 +480,15 @@ include::includes/cmd-config-section-all.txt[]\n \n include::config/sendemail.txt[]\n \n+\n+EXIT STATUS\n+-----------\n+\n+Zero is returned when all specified patches were sent, while 1 is returned\n+when an error occurs. 10 is returned if the user interactively skips sending\n+only some patches, and 11 if they skip all patches.\n+\n+\n EXAMPLES\n --------\n Use gmail as the smtp server\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex affbb88509..2c62de07bc 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -256,6 +256,26 @@ sub system_or_die {\n \tdie $msg if $msg;\n }\n \n+my $sent_files = 0;\n+\n+sub do_exit {\n+\tif ($sent_files == @files) {\n+\t\t# All specified messages were sent\n+\t\texit(0);\n+\t} elsif ($sent_files) {\n+\t\t# At least some messages were sent\n+\t\texit(10);\n+\t} else {\n+\t\t# User skipped all messages or quit before sending the first one\n+\t\texit(11);\n+\t}\n+}\n+\n+sub do_quit {\n+\tcleanup_compose_files();\n+\tdo_exit();\n+}\n+\n sub do_edit {\n \tif (!defined($editor)) {\n \t\t$editor = Git::command_oneline('var', 'GIT_EDITOR');\n@@ -1195,8 +1215,7 @@ sub validate_address {\n \t\tif (/^d/i) {\n \t\t\treturn undef;\n \t\t} elsif (/^q/i) {\n-\t\t\tcleanup_compose_files();\n-\t\t\texit(0);\n+\t\t\tdo_quit();\n \t\t}\n \t\t$address = ask(\"$to_whom \",\n \t\t\tdefault => \"\",\n@@ -1619,8 +1638,7 @@ sub send_message {\n \t\t} elsif (/^e/i) {\n \t\t\treturn -1;\n \t\t} elsif (/^q/i) {\n-\t\t\tcleanup_compose_files();\n-\t\t\texit(0);\n+\t\t\tdo_quit();\n \t\t} elsif (/^a/i) {\n \t\t\t$confirm = 'never';\n \t\t}\n@@ -2001,6 +2019,10 @@ sub process_file {\n \t\treturn 0;\n \t}\n \n+\tif ($message_was_sent) {\n+\t\t$sent_files++;\n+\t}\n+\n \t# set up for the next message\n \tif ($thread) {\n \t\tif ($message_was_sent &&\n@@ -2278,3 +2300,5 @@ sub body_or_subject_has_nonascii {\n \t}\n \treturn 0;\n }\n+\n+do_exit();\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 48bf0af2ee..64f9c7c154 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1187,6 +1187,42 @@ test_expect_success $PREREQ 'confirm does not loop forever' '\n \t\t\t$patches\n '\n \n+test_confirm_many () {\n+\tclean_fake_sendmail\n+\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\tgit send-email \\\n+\t\t--confirm=auto \\\n+\t\t--from=\"Example <nobody@example.com>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t-2 <replies\n+\techo $? >exit_sts\n+\ttest_cmp expected_sts exit_sts || return 1\n+\tls commandline* 2>/dev/null | wc -l >num_mails\n+\ttest_cmp expected_num num_mails || return 1\n+}\n+\n+test_expect_success $PREREQ 'interactively skip none' '\n+\t(echo y && echo y) >replies &&\n+\techo 0 >expected_sts &&\n+\techo 2 >expected_num &&\n+\ttest_confirm_many\n+'\n+\n+test_expect_success $PREREQ 'interactively skip some' '\n+\t(echo n && echo y) >replies &&\n+\techo 10 >expected_sts &&\n+\techo 1 >expected_num &&\n+\ttest_confirm_many\n+'\n+\n+test_expect_success $PREREQ 'interactively skip all' '\n+\t(echo n && echo n) >replies &&\n+\techo 11 >expected_sts &&\n+\techo 0 >expected_num &&\n+\ttest_confirm_many\n+'\n+\n test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '\n \tclean_fake_sendmail &&\n \trm -fr outdir &&\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480849","messageId":"xmqq1qfwqnwu.fsf@gitster.g","threadId":"59658","inReplyTo":"20230821170720.577835-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v5] send-email: prompt-dependent exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-21T17:57:05Z","receivedAt":"2023-08-21T17:57:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> to opt-in (which is obviously the status quo, and is likely to persist\n> in most cases) is potentially at least somewhat serious.\n\nWhat's the weaseling \"potentially at least somewhat\" phrase about?\n\nQuite honestly, it makes it apparent that the patch is trying to\nmake a change that cannot be justified and makes it lose all the\nremaining credibility.\n\nThe users have lived with the current behaviour practically forever\nin Git's timescale and not changing the default and letting them\n\"live with\" the status quo cannot be any _serious_ at all.\n\nWill replace what I had kept but out of 'seen', as the changed parts\nof the patch are definite improvements, but I do not think I want to\nmerge it without starting this as an optional feature.\n\nThanks.\n\n"},{"id":"480855","messageId":"ZOOzrHWLchYMgUOd@ugly","threadId":"59658","inReplyTo":"xmqq1qfwqnwu.fsf@gitster.g","subject":"Re: [PATCH v5] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-21T18:57:48Z","receivedAt":"2023-08-21T18:58:46Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Mon, Aug 21, 2023 at 10:57:05AM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> to opt-in (which is obviously the status quo, and is likely to persist\n>> in most cases) is potentially at least somewhat serious.\n>\n>What's the weaseling \"potentially at least somewhat\" phrase about?\n>\ni think that mails not being sent (timely, until somebody notices the \nmistake) is \"at least somewhat serious\". at least serious enough to \njustify a few people having to make a minor ajustment to their scripts \nto confirm that they really want the old behavior.\n\nregards\n"},{"id":"481167","messageId":"xmqq3501bbmf.fsf@gitster.g","threadId":"59658","inReplyTo":"20230821170720.577835-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v5] send-email: prompt-dependent exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-30T00:46:32Z","receivedAt":"2023-08-30T00:47:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> Proposed content for RelNotes:\n>\n>  * \"git send-email\" now reports interactive cancellation via a distinct\n>    non-zero exit status. Callers which do not consider this situation an\n>    error need to be adjusted.\n\nIf we still want to deliberately break folks with this change, we\nshould not blame users for becoming accustomed to the traditional\nbehaviour and pretend as if burdening them to change their scripts\nis within our rights.  We should be a lot more apologetic in the\nbackward compatibility notes than what you wrote in the above.\n\nHaving said that, quite honestly, I do not think this new behaviour\ndeserves to be a new default from the day one, with need for an\napology to existing users by the project.  Given that the users have\nlived with the current behaviour practically forever in Git's\ntimescale and that not changing the default and letting them \"live\nwith\" the status quo cannot cause any serious problem, I cannot\nstand behind such a default change myself.  It should be a new\nfeature that is opt-in, just like any other new and useful feature..\n\nBy the way, I just noticed that the test will not pass on BSD\nderived systems.  Something like this need to be squashed in if we\nwant to proceed further.\n\n--- >8 --- squash --- >8 ---\nSubject: [PATCH] SQUASH???\n\nSending \"wc -l\" output to a file and expecting that it has a run of\ndigits with terminating newline and nothing else is prone to break\non BSD derived systems, including macOS.\n---\n t/t9001-send-email.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 64f9c7c154..8323783963 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1198,7 +1198,7 @@ test_confirm_many () {\n \t\t-2 <replies\n \techo $? >exit_sts\n \ttest_cmp expected_sts exit_sts || return 1\n-\tls commandline* 2>/dev/null | wc -l >num_mails\n+\techo $(ls commandline* 2>/dev/null | wc -l) >num_mails\n \ttest_cmp expected_num num_mails || return 1\n }\n \n-- \n2.42.0-81-g1a190bc14a\n\n"},{"id":"481180","messageId":"ZO8Ut3t2ns1lj/pM@ugly","threadId":"59658","inReplyTo":"xmqq3501bbmf.fsf@gitster.g","subject":"Re: [PATCH v5] send-email: prompt-dependent exit codes","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-30T10:06:47Z","receivedAt":"2023-08-30T18:29:33Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Tue, Aug 29, 2023 at 05:46:32PM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> Proposed content for RelNotes:\n>>\n>>  * \"git send-email\" now reports interactive cancellation via a distinct\n>>    non-zero exit status. Callers which do not consider this situation an\n>>    error need to be adjusted.\n>\n>If we still want to deliberately break folks with this change, we\n>should not blame users for becoming accustomed to the traditional\n>behaviour and pretend as if burdening them to change their scripts\n>is within our rights.  We should be a lot more apologetic in the\n>backward compatibility notes than what you wrote in the above.\n>\ni find this deference mind-boggling.\n\ni'm sure you're somewhat cautious because you're on the frontline when \npeople complain, and *someone* will _always_ complain when things \nchange.  however, it's completely unreasonable to let this dictate one's \npossibilities. there is a trade-off between the cost of change and the \ncost of non-change.\n\n>Given that the users have\n>lived with the current behaviour practically forever in Git's\n>timescale and that not changing the default and letting them \"live\n>with\" the status quo cannot cause any serious problem, I cannot\n>stand behind such a default change myself.\n>\nthe exit code really only matters when there is a wrapper which decides \nsignificant follow-up actions based on it (that is, doesn't exit right \nafterwards anyway). i don't think many such scripts exist for \ngit-send-email. those which do will most likely suppress interaction.\nwhich is why nobody else complained yet. and also why likely nobody will \ncomplain if we change it. the few people that will notice at all will \nmost likely welcome the change, as i would.\n\n>It should be a new feature that is opt-in,\n>\ndo as you like, but i won't spend the time on roughly doubling the size \nof the patch, nor on dealing with this being an opt-in (i don't want a \nhard dep on a new git version in the near term), and i won't contribute \nto sabotaging the discovery of hidden bugs.\n\n>just like any other new and useful feature..\n>\nnot reporting a significant event is a bug.\n\n>Subject: [PATCH] SQUASH???\n>\nyes, please.\n\nregards\n"}]}