Samuel GROOT [off-list ref] writes:
If used with `in-reply-to=<email_file>`, cite the message body of the given
email file. Otherwise, do nothing.
It should at least warn when --in-reply-to=<email_file> is not given
(either no --in-reply-to or --in-reply-to=<id>). I don't see any
use-case where a user would want --cite on the command-line and not want
--in-reply-to=<email_file>. OTOH, it seems a plausible user-error, and
the user would appreciate a message saying what's going on.
quoted hunk
@@ -56,6 +57,8 @@ git send-email --dump-aliases
--subject <str> * Email "Subject:"
--in-reply-to <str> * Email "In-Reply-To:"
--in-reply-to <file> * Populate header fields appropriately.
+ --cite * Quote the message body in the cover if
+ --compose is set, else in the first patch.
--[no-]xmailer * Add "X-Mailer:" header (default).
--[no-]annotate * Review each patch that will be sent in an editor.
--compose * Open an editor for introduction.
Just wondering: would it make sense to activate --cite by default when
--in-reply-to=file is used, and to allow --no-cite to disable this?
This is something we can easily do now without breaking backward
compatibility (--in-reply-to=file doesn't exist yet), but would be more
painful to do later.
quoted hunk
@@ -640,6 +644,7 @@ if (@files) {
usage();
}
+my $message_cited;
Nit: I read "$message_cited" as "Boolean saying whether the message was
cited". $cited_message would be clearer to me (but this is to be taken
with a grain of salt as I'm not a native speaker), since the variable
holds the content of the cited message.
+sub do_insert_cited_message {
+ my $tmp_file = shift;
+ my $original_file = shift;
+
+ open my $c, "<", $original_file
+ or die "Failed to open $original_file: " . $!;
+
+ open my $c2, ">", $tmp_file
+ or die "Failed to open $tmp_file: " . $!;
+
+ # Insertion after the triple-dash
+ while (<$c>) {
+ print $c2 $_;
+ last if (/^---$/);
+ }
+ print $c2 $message_cited;
I would add a newline here to get a blank line between the message cited
and the diffstat.
I think non-ascii characters would deserve particular attention here
too. For example, if the patch contain only ascii and the cited part
contains UTF-8, does the generated patch have a proper Content-type:
header?
I can imagine worse, like a patch containing latin1 character and a
cited message with another 8-bit encoding.
+test_expect_success $PREREQ 'correct cited message with --in-reply-to and --compose' '
+ grep "> On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:" msgtxt3 &&
I would prefer to have the full address including the real name here (A
[off-list ref]) in this example. Actually, after a quick look at
the code, I don't understand where the name has gone (what's shown here
is extracted from the From: header).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
On 06/09/2016 01:49 PM, Matthieu Moy wrote:
Samuel GROOT [off-list ref] writes:
quoted
If used with `in-reply-to=<email_file>`, cite the message body of the given
email file. Otherwise, do nothing.
It should at least warn when --in-reply-to=<email_file> is not given
(either no --in-reply-to or --in-reply-to=<id>). I don't see any
use-case where a user would want --cite on the command-line and not want
--in-reply-to=<email_file>. OTOH, it seems a plausible user-error, and
the user would appreciate a message saying what's going on.
We weren't sure how to warn the user.
If --in-reply-to is not an mail file, should we check it with a regex to
make sure it's a message-id?
quoted
@@ -56,6 +57,8 @@ git send-email --dump-aliases
--subject <str> * Email "Subject:"
--in-reply-to <str> * Email "In-Reply-To:"
--in-reply-to <file> * Populate header fields appropriately.
+ --cite * Quote the message body in the cover if
+ --compose is set, else in the first patch.
--[no-]xmailer * Add "X-Mailer:" header (default).
--[no-]annotate * Review each patch that will be sent in an editor.
--compose * Open an editor for introduction.
Just wondering: would it make sense to activate --cite by default when
--in-reply-to=file is used, and to allow --no-cite to disable this?
This is something we can easily do now without breaking backward
compatibility (--in-reply-to=file doesn't exist yet), but would be more
painful to do later.
It's an interesting question.
IMHO we should have `--cite` by default and allow `--no-cite` to disable
quoting the message body, because it's easier to remove extra unwanted
lines than copying lines from another file and adding "> " before each line.
quoted
@@ -640,6 +644,7 @@ if (@files) {
usage();
}
+my $message_cited;
Nit: I read "$message_cited" as "Boolean saying whether the message was
cited". $cited_message would be clearer to me (but this is to be taken
with a grain of salt as I'm not a native speaker), since the variable
holds the content of the cited message.
quoted
+sub do_insert_cited_message {
+ my $tmp_file = shift;
+ my $original_file = shift;
+
+ open my $c, "<", $original_file
+ or die "Failed to open $original_file: " . $!;
+
+ open my $c2, ">", $tmp_file
+ or die "Failed to open $tmp_file: " . $!;
+
+ # Insertion after the triple-dash
+ while (<$c>) {
+ print $c2 $_;
+ last if (/^---$/);
+ }
+ print $c2 $message_cited;
I would add a newline here to get a blank line between the message cited
and the diffstat.
A newline here makes it easier to distinguish the different sections in
the email file indeed.
I think non-ascii characters would deserve particular attention here
too. For example, if the patch contain only ascii and the cited part
contains UTF-8, does the generated patch have a proper Content-type:
header?
Non-ascii characters are still an issue, I'll work on that next week.
I can imagine worse, like a patch containing latin1 character and a
cited message with another 8-bit encoding.
quoted
+test_expect_success $PREREQ 'correct cited message with --in-reply-to and --compose' '
+ grep "> On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:" msgtxt3 &&
I would prefer to have the full address including the real name here (A
[off-list ref]) in this example. Actually, after a quick look at
the code, I don't understand where the name has gone (what's shown here
is extracted from the From: header).
Yep, after a quick look at the code involved, I don't understand either,
I will investigate this week.
Le 09/06/2016 à 13:49, Matthieu Moy a écrit :
Samuel GROOT [off-list ref] writes:
quoted
If used with `in-reply-to=<email_file>`, cite the message body of the given
email file. Otherwise, do nothing.
It should at least warn when --in-reply-to=<email_file> is not given
(either no --in-reply-to or --in-reply-to=<id>). I don't see any
use-case where a user would want --cite on the command-line and not want
--in-reply-to=<email_file>. OTOH, it seems a plausible user-error, and
the user would appreciate a message saying what's going on.
You're right. We'll warn the user with the next version.
quoted
@@ -56,6 +57,8 @@ git send-email --dump-aliases
--subject <str> * Email "Subject:"
--in-reply-to <str> * Email "In-Reply-To:"
--in-reply-to <file> * Populate header fields appropriately.
+ --cite * Quote the message body in the cover if
+ --compose is set, else in the first patch.
--[no-]xmailer * Add "X-Mailer:" header (default).
--[no-]annotate * Review each patch that will be sent in an editor.
--compose * Open an editor for introduction.
Just wondering: would it make sense to activate --cite by default when
--in-reply-to=file is used, and to allow --no-cite to disable this?
This is something we can easily do now without breaking backward
compatibility (--in-reply-to=file doesn't exist yet), but would be more
painful to do later.
Indeed, it can be more intuitive especially for a user who is used to
cite messages.
quoted
@@ -640,6 +644,7 @@ if (@files) {
usage();
}
+my $message_cited;
Nit: I read "$message_cited" as "Boolean saying whether the message was
cited". $cited_message would be clearer to me (but this is to be taken
with a grain of salt as I'm not a native speaker), since the variable
holds the content of the cited message.
Sorry, French habits.. :-)
quoted
+sub do_insert_cited_message {
+ my $tmp_file = shift;
+ my $original_file = shift;
+
+ open my $c, "<", $original_file
+ or die "Failed to open $original_file: " . $!;
+
+ open my $c2, ">", $tmp_file
+ or die "Failed to open $tmp_file: " . $!;
+
+ # Insertion after the triple-dash
+ while (<$c>) {
+ print $c2 $_;
+ last if (/^---$/);
+ }
+ print $c2 $message_cited;
I would add a newline here to get a blank line between the message cited
and the diffstat.
Agreed.
I think non-ascii characters would deserve particular attention here
too. For example, if the patch contain only ascii and the cited part
contains UTF-8, does the generated patch have a proper Content-type:
header?
I can imagine worse, like a patch containing latin1 character and a
cited message with another 8-bit encoding.
I tried to manage them with the built-in Base64 module but there is
still work in progress.
quoted
+test_expect_success $PREREQ 'correct cited message with --in-reply-to and --compose' '
+ grep "> On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:" msgtxt3 &&
I would prefer to have the full address including the real name here (A
[off-list ref]) in this example. Actually, after a quick look at
the code, I don't understand where the name has gone (what's shown here
is extracted from the From: header).
Agreed, I'll figure out where the problem is.