threads / patch / 20410

patchsend-email: confirm on empty mail subjects

Subject: [PATCH] send-email: confirm on empty mail subjects

## tl;dr

7 messages between Aug 5, 2009 and Sep 11, 2009. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Jan Engelhardt· Aug 5, 2009, 16:49 UTC · lore

When the user forgot to enter a subject in a compose session, send-email will now inquire whether this is really intended, similar to what the Alpine MUA does when a subject is absent.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 git-send-email.perl |   25 ++++++++++++++++++++-----
 1 files changed, 20 insertions(+), 5 deletions(-)
Show changes to git-send-email.perl +20 −5
diff --git a/git-send-email.perl b/git-send-email.perl
index d508f83..7d56fba 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -553,11 +553,26 @@ EOT
 	}
 	close(C);
 
-	if ($annotate) {
-		do_edit($compose_filename, @files);
-	} else {
-		do_edit($compose_filename);
-	}
+	my $re_edit = 0;
+	do {
+		if ($annotate) {
+			do_edit($compose_filename, @files);
+		} else {
+			do_edit($compose_filename);
+		}
+
+		open(C, "<", $compose_filename) ||
+			die "Failed to open $compose_filename: $!";
+		if (grep(/^Subject:\s*$/i, <C>)) {
+			my $r = ask("No Subject, send anyway? ".
+			            "([y]es|[n]o|[e]dit again): ",
+			            valid_re => qr/^[yne]/i,
+			            default => "n");
+			$re_edit = lc(substr($r, 0, 1)) eq "e";
+			exit(0) if lc(substr($r, 0, 1)) eq "n";
+		}
+		close C;
+	} while ($re_edit);
 
 	open(C2,">",$compose_filename . ".final")
 		or die "Failed to open $compose_filename.final : " . $!;

-- 
1.6.4
Junio C Hamano· Aug 6, 2009, 06:25 UTC · re: Jan Engelhardt · lore

Re: [PATCH] send-email: confirm on empty mail subjects

Jan Engelhardt <jengelh@medozas.de> writes:
> When the user forgot to enter a subject in a compose session,
> send-email will now inquire whether this is really intended, similar
> to what the Alpine MUA does when a subject is absent.
This seems to break t9001...
Jan Engelhardt· Aug 24, 2009, 17:27 UTC · re: Junio C Hamano · lore

Re: [PATCH] send-email: confirm on empty mail subjects

On Thursday 2009-08-06 08:25, Junio C Hamano wrote:
Show 8 quoted lines
>Jan Engelhardt <jengelh@medozas.de> writes:
>
>> When the user forgot to enter a subject in a compose session,
>> send-email will now inquire whether this is really intended, similar
>> to what the Alpine MUA does when a subject is absent.
>
>This seems to break t9001...
>
Did I miss something in building?

19:26 sovereign:../git/git-1.6.4.1 > quilt pu Applying patch patches/send-email-empty-subject.diff patching file git-send-email.perl

Now at patch patches/send-email-empty-subject.diff
19:26 sovereign:../git/git-1.6.4.1 > cd t
19:26 sovereign:../git-1.6.4.1/t > ./t9001-send-email.sh 
*   ok 1: prepare reference tree
*   ok 2: Setup helper tool
*   ok 3: Extract patches
*   ok 4: No confirm with --suppress-cc
*   ok 5: No confirm with --confirm=never
*   ok 6: No confirm with sendemail.confirm=never
*   ok 7: Send patches
*   ok 8: Verify commandline
*   ok 9: Show all headers
*   ok 10: Prompting works
*   ok 11: cccmd works
*   ok 12: reject long lines
*   ok 13: no patch was sent
*   ok 14: Author From: in message body
*   ok 15: Author From: not in message body
*   ok 16: allow long lines with --no-validate
*   ok 17: Invalid In-Reply-To
*   ok 18: Valid In-Reply-To when prompting
*   ok 19: setup fake editor
*   ok 20: --compose works
*   ok 21: first message is compose text
*   ok 22: second message is patch
*   ok 23: sendemail.cc set
*   ok 24: sendemail.cc unset
*   ok 25: sendemail.cccmd
*   ok 26: --suppress-cc=all
*   ok 27: --suppress-cc=body
*   ok 28: --suppress-cc=body --suppress-cc=cccmd
*   ok 29: --suppress-cc=sob
*   ok 30: --suppress-cc=bodycc
*   ok 31: --suppress-cc=cc
*   ok 32: --confirm=always
*   ok 33: --confirm=auto
*   ok 34: --confirm=cc
*   ok 35: --confirm=compose
*   ok 36: confirm by default (due to cc)
*   ok 37: confirm by default (due to --compose)
*   ok 38: confirm detects EOF (inform assumes y)
*   ok 39: confirm detects EOF (auto causes failure)
*   ok 40: confirm doesnt loop forever
*   ok 41: utf8 Cc is rfc2047 encoded
*   ok 42: --compose adds MIME for utf8 body
*   ok 43: --compose respects user mime type
*   ok 44: --compose adds MIME for utf8 subject
*   ok 45: detects ambiguous reference/file conflict
*   ok 46: feed two files
*   ok 47: in-reply-to but no threading
*   ok 48: no in-reply-to and no threading
*   ok 49: threading but no chain-reply-to
* passed all 49 test(s)
Junio C Hamano· Aug 24, 2009, 18:19 UTC · re: Jan Engelhardt · lore

Re: [PATCH] send-email: confirm on empty mail subjects

Jan Engelhardt <jengelh@medozas.de> writes:
Show 16 quoted lines
> On Thursday 2009-08-06 08:25, Junio C Hamano wrote:
>
>>Jan Engelhardt <jengelh@medozas.de> writes:
>>
>>> When the user forgot to enter a subject in a compose session,
>>> send-email will now inquire whether this is really intended, similar
>>> to what the Alpine MUA does when a subject is absent.
>>
>>This seems to break t9001...
>>
>
> Did I miss something in building?
>
> 19:26 sovereign:../git/git-1.6.4.1 > quilt pu
> Applying patch patches/send-email-empty-subject.diff
> patching file git-send-email.perl

Is this using 'pu' with your patch? Near the tip of the 'pu' branch I have a iffy workaround to "unbreak" the issue, but it is a rather sledgehammer approach I do not feel comfortable enough to squash into your patch yet.

Jan Engelhardt· Aug 25, 2009, 16:27 UTC · re: Junio C Hamano · lore

Re: [PATCH] send-email: confirm on empty mail subjects

On Montag 2009-08-24 20:19, Junio C Hamano wrote:
Show 14 quoted lines
>>>
>>>> When the user forgot to enter a subject in a compose session,
>>>> send-email will now inquire whether this is really intended, similar
>>>> to what the Alpine MUA does when a subject is absent.
>>>
>>>This seems to break t9001...
>>
>> Did I miss something in building?
>>
>> 19:26 sovereign:../git/git-1.6.4.1 > quilt pu
>> Applying patch patches/send-email-empty-subject.diff
>> patching file git-send-email.perl
>
>Is this using 'pu' with your patch?
Ah no, `quilt pu` is an autoalias for `quilt push`.
>Near the tip of the 'pu' branch I
>have a iffy workaround to "unbreak" the issue, but it is a rather
>sledgehammer approach I do not feel comfortable enough to squash into your
>patch yet.
I see. Perhaps
	echo -en 'y\ny\n' | ...

would be more gentle? (Noting that, how else should it be, many a shell do not have -e/-n again.)

Junio C Hamano· Aug 25, 2009, 17:35 UTC · re: Jan Engelhardt · lore

Re: [PATCH] send-email: confirm on empty mail subjects

Jan Engelhardt <jengelh@medozas.de> writes:
Show 11 quoted lines
>>Near the tip of the 'pu' branch I
>>have a iffy workaround to "unbreak" the issue, but it is a rather
>>sledgehammer approach I do not feel comfortable enough to squash into your
>>patch yet.
>
> I see. Perhaps
>
> 	echo -en 'y\ny\n' | ...
>
> would be more gentle? (Noting that, how else should it be,
> many a shell do not have -e/-n again.)

You can solve it with printf "y\ny\n", but the reason I said it feels wrong was because your added tests are the _only_ ones that expect more than one "yes".

If some _other_ tests that currently need only one "yes" are broken in the future and starts asking for more than one, we would like to know about the breakage, but we won't notice it if we unconditionally fed "yes | ..." or your "two y's | ..." to them. That is what I am unhappy about the "iffy workaround".

Junio C Hamano· Sep 11, 2009, 23:27 UTC · re: Jan Engelhardt · lore

Re: [PATCH] send-email: confirm on empty mail subjects

Jan Engelhardt <jengelh@medozas.de> writes:
Show 8 quoted lines
> When the user forgot to enter a subject in a compose session,
> send-email will now inquire whether this is really intended, similar
> to what the Alpine MUA does when a subject is absent.
>
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
>  git-send-email.perl |   25 ++++++++++++++++++++-----
>  1 files changed, 20 insertions(+), 5 deletions(-)

We have had this for quite a long time but I have two niggling worries, one minor, another one showstopper from maintainability point of view.

 - With --confirm=never the program still seems to talk with the terminal.
   I think with --confirm=neber we should not ask but just fail.
 - This does not hook into the confirmation framework the program already
   has, and does not have any way to turn it off.
Show 38 quoted lines
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d508f83..7d56fba 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -553,11 +553,26 @@ EOT
>  	}
>  	close(C);
>  
> -	if ($annotate) {
> -		do_edit($compose_filename, @files);
> -	} else {
> -		do_edit($compose_filename);
> -	}
> +	my $re_edit = 0;
> +	do {
> +		if ($annotate) {
> +			do_edit($compose_filename, @files);
> +		} else {
> +			do_edit($compose_filename);
> +		}
> +
> +		open(C, "<", $compose_filename) ||
> +			die "Failed to open $compose_filename: $!";
> +		if (grep(/^Subject:\s*$/i, <C>)) {
> +			my $r = ask("No Subject, send anyway? ".
> +			            "([y]es|[n]o|[e]dit again): ",
> +			            valid_re => qr/^[yne]/i,
> +			            default => "n");
> +			$re_edit = lc(substr($r, 0, 1)) eq "e";
> +			exit(0) if lc(substr($r, 0, 1)) eq "n";
> +		}
> +		close C;
> +	} while ($re_edit);
>  
>  	open(C2,">",$compose_filename . ".final")
>  		or die "Failed to open $compose_filename.final : " . $!;
> -- 
> 1.6.4

← back to recent threads