{"thread":{"id":"48409","subject":"[PATCHv2] git-send-email: allow re-editing of message","startedAt":"2018-05-04T13:08:27Z","lastAt":"2018-05-06T03:56:30Z","messageCount":2,"participants":["Drew DeVault","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"346647","messageId":"20180504130811.3398-1-sir@cmpwn.com","threadId":"48409","inReplyTo":null,"subject":"[PATCHv2] git-send-email: allow re-editing of message","fromName":"Drew DeVault","fromEmail":"sir@cmpwn.com","sentAt":"2018-05-04T13:08:11Z","receivedAt":"2018-05-04T13:08:27Z","isPatch":false,"sender":{"key":"sir@cmpwn.com","avatar":"https://avatars.githubusercontent.com/u/1310872?v=4"},"body":"When shown the email summary, an opportunity is presented for the user\nto edit the email as if they had specified --annotate. This also permits\nthem to edit it multiple times.\n\nSigned-off-by: Drew DeVault <sir@cmpwn.com>\nReviewed-by: Simon Ser <contact@emersion.fr>\n\n---\nThanks for the review Eric, updated to address your feedback.\n\n git-send-email.perl | 38 +++++++++++++++++++++++++++++++-------\n 1 file changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2fa7818ca..b45953733 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1330,9 +1330,14 @@ sub file_name_is_absolute {\n \treturn File::Spec::Functions::file_name_is_absolute($path);\n }\n \n-# Returns 1 if the message was sent, and 0 otherwise.\n-# In actuality, the whole program dies when there\n-# is an error sending a message.\n+# Prepares the email, then asks the user what to do.\n+#\n+# If the user chooses to send the email, it's sent and 1 is returned.\n+# If the user chooses not to send the email, 0 is returned.\n+# If the user decides they want to make further edits, -1 is returned and the\n+# caller is expected to call send_message again after the edits are performed.\n+#\n+# If an error occurs sending the email, this just dies.\n \n sub send_message {\n \tmy @recipients = unique_email_list(@to);\n@@ -1404,15 +1409,17 @@ Message-Id: $message_id\n \n EOF\n \t\t}\n-\t\t# TRANSLATORS: Make sure to include [y] [n] [q] [a] in your\n+\t\t# TRANSLATORS: Make sure to include [y] [n] [e] [q] [a] in your\n \t\t# translation. The program will only accept English input\n \t\t# at this point.\n-\t\t$_ = ask(__(\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \"),\n-\t\t         valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,\n+\t\t$_ = ask(__(\"Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): \"),\n+\t\t         valid_re => qr/^(?:yes|y|no|n|edit|e|quit|q|all|a)/i,\n \t\t         default => $ask_default);\n \t\tdie __(\"Send this email reply required\") unless defined $_;\n \t\tif (/^n/i) {\n \t\t\treturn 0;\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@@ -1552,7 +1559,12 @@ $references = $initial_in_reply_to || '';\n $subject = $initial_subject;\n $message_num = 0;\n \n-foreach my $t (@files) {\n+# Prepares the email, prompts the user, sends it out\n+# Returns 0 if an edit was done and the function should be called again, or 1\n+# otherwise.\n+sub process_file {\n+\tmy ($t) = @_;\n+\n \topen my $fh, \"<\", $t or die sprintf(__(\"can't open file %s\"), $t);\n \n \tmy $author = undef;\n@@ -1755,6 +1767,10 @@ foreach my $t (@files) {\n \t}\n \n \tmy $message_was_sent = send_message();\n+\tif ($message_was_sent == -1) {\n+\t\tdo_edit($t);\n+\t\treturn 0;\n+\t}\n \n \t# set up for the next message\n \tif ($thread && $message_was_sent &&\n@@ -1776,6 +1792,14 @@ foreach my $t (@files) {\n \t\tundef $auth;\n \t\tsleep($relogin_delay) if defined $relogin_delay;\n \t}\n+\n+\treturn 1;\n+}\n+\n+foreach my $t (@files) {\n+\twhile (!process_file($t)) {\n+\t\t# user edited the file\n+\t}\n }\n \n # Execute a command (e.g. $to_cmd) to get a list of email addresses\n-- \n2.17.0\n\n"},{"id":"346745","messageId":"xmqq4ljlsahj.fsf@gitster-ct.c.googlers.com","threadId":"48409","inReplyTo":"20180504130811.3398-1-sir@cmpwn.com","subject":"Re: [PATCHv2] git-send-email: allow re-editing of message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-06T03:56:24Z","receivedAt":"2018-05-06T03:56:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Drew DeVault <sir@cmpwn.com> writes:\n\n> When shown the email summary, an opportunity is presented for the user\n> to edit the email as if they had specified --annotate. This also permits\n> them to edit it multiple times.\n>\n> Signed-off-by: Drew DeVault <sir@cmpwn.com>\n> Reviewed-by: Simon Ser <contact@emersion.fr>\n>\n> ---\n> Thanks for the review Eric, updated to address your feedback.\n\nInstead you could credit him with Helped-by:, perhaps in place of\nReviewed-by: somebody who does not have any commit in our history,\nwhich does not help others decide how good this patch is because\nthey do not know how much trust they should place in the ability to\nreview of somebody they never have heard of---Simon may be a super\nhuman programmer whose name alone should assure us that a patch\nendorsed is good, but the thing is, that won't happen until we know\nthat.\n\nThe patch looks good.  Eric may say \"This round looks good to me\"\nbefore I have a chance to queue it, in which case I'll add his\nReviewed-by: and queue.  In either case, unless there is something\nelse comes up, there is no need to resend this patch.\n\nThanks.\n\n>  git-send-email.perl | 38 +++++++++++++++++++++++++++++++-------\n>  1 file changed, 31 insertions(+), 7 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 2fa7818ca..b45953733 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1330,9 +1330,14 @@ sub file_name_is_absolute {\n>  \treturn File::Spec::Functions::file_name_is_absolute($path);\n>  }\n>  \n> -# Returns 1 if the message was sent, and 0 otherwise.\n> -# In actuality, the whole program dies when there\n> -# is an error sending a message.\n> +# Prepares the email, then asks the user what to do.\n> +#\n> +# If the user chooses to send the email, it's sent and 1 is returned.\n> +# If the user chooses not to send the email, 0 is returned.\n> +# If the user decides they want to make further edits, -1 is returned and the\n> +# caller is expected to call send_message again after the edits are performed.\n> +#\n> +# If an error occurs sending the email, this just dies.\n>  \n>  sub send_message {\n>  \tmy @recipients = unique_email_list(@to);\n> @@ -1404,15 +1409,17 @@ Message-Id: $message_id\n>  \n>  EOF\n>  \t\t}\n> -\t\t# TRANSLATORS: Make sure to include [y] [n] [q] [a] in your\n> +\t\t# TRANSLATORS: Make sure to include [y] [n] [e] [q] [a] in your\n>  \t\t# translation. The program will only accept English input\n>  \t\t# at this point.\n> -\t\t$_ = ask(__(\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \"),\n> -\t\t         valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,\n> +\t\t$_ = ask(__(\"Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): \"),\n> +\t\t         valid_re => qr/^(?:yes|y|no|n|edit|e|quit|q|all|a)/i,\n>  \t\t         default => $ask_default);\n>  \t\tdie __(\"Send this email reply required\") unless defined $_;\n>  \t\tif (/^n/i) {\n>  \t\t\treturn 0;\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> @@ -1552,7 +1559,12 @@ $references = $initial_in_reply_to || '';\n>  $subject = $initial_subject;\n>  $message_num = 0;\n>  \n> -foreach my $t (@files) {\n> +# Prepares the email, prompts the user, sends it out\n> +# Returns 0 if an edit was done and the function should be called again, or 1\n> +# otherwise.\n> +sub process_file {\n> +\tmy ($t) = @_;\n> +\n>  \topen my $fh, \"<\", $t or die sprintf(__(\"can't open file %s\"), $t);\n>  \n>  \tmy $author = undef;\n> @@ -1755,6 +1767,10 @@ foreach my $t (@files) {\n>  \t}\n>  \n>  \tmy $message_was_sent = send_message();\n> +\tif ($message_was_sent == -1) {\n> +\t\tdo_edit($t);\n> +\t\treturn 0;\n> +\t}\n>  \n>  \t# set up for the next message\n>  \tif ($thread && $message_was_sent &&\n> @@ -1776,6 +1792,14 @@ foreach my $t (@files) {\n>  \t\tundef $auth;\n>  \t\tsleep($relogin_delay) if defined $relogin_delay;\n>  \t}\n> +\n> +\treturn 1;\n> +}\n> +\n> +foreach my $t (@files) {\n> +\twhile (!process_file($t)) {\n> +\t\t# user edited the file\n> +\t}\n>  }\n>  \n>  # Execute a command (e.g. $to_cmd) to get a list of email addresses\n"}]}