Thread (9 messages) flat view 9 messages, 3 authors, 2016-06-15

Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:38

Possibly related (same subject, not in this thread)

xzer [off-list ref] writes:
Subject: Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.
We prefer to have "[PATCH] subsystem: description without final full-stop" here.
There is still a problem that git-am will lost the line break.
What does "still" refer to?  It is unclear under what condition the
command lose "the line break" (nor which line break you are refering to; I
am guessing that you have a commit that begins with a multi-line paragraph
and you are talking about line breaks between the lines in the first
paragraph).
It's not easy to retain it, but as the first step, we can generate
a valid rfc2047 header now.
Please describe what is broken (iow, "Given this sample input, we
currently generate this output, which is not a valid rfc2047") and what
the new output looks like ("Update pp_title_line() to generate this output
instead.")
---
Missing sign-off with a real name.
quoted hunk
diff --git a/pretty.c b/pretty.c
index 8549934..f18a38d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -249,6 +249,33 @@ needquote:
 	strbuf_addstr(sb, "?=");
 }
 
+static void add_rfc2047_multiline(struct strbuf *sb, const char *line, int len,
+                       const char *encoding)
+{
+	int first = 1;
+	char *mline = xmemdupz(line, len);
+	const char *cline = mline;
+	int offset = 0, linelen = 0;
+        for (;;) {
You seem to have indent that uses SPs instead of HT around here...
+                linelen = get_one_line(cline);
I can see you are trying to be careful not to let get_one_line() overstep
past "len" the caller gave you by making a copy first, but is this
overhead really necessary?  After all we know in this static function that
the caller is feeding the contents from a strbuf, which always have a
terminating NUL (and that is why it is Ok that get_one_line() is not a
counted string interface).
+
+                cline += linelen;
+
+                if (!linelen)
+                        break;
+		
+		if (!first)
+			strbuf_addf(sb, "\n ");
+
+		offset = *(cline -1) == '\n'; 
+
+		add_rfc2047(sb, cline-linelen, linelen-offset, encoding);
+		first = 0;
+
+        }
+	free(mline);
+}
So the general idea of this change (I am thinking aloud what should be in
the updated commit log message as the problem description) is that:

 - We currently give an entire multi-line paragraph string to the
   add_rfc2047() function to be formatted as the title of the commit;

 - The add_rfc2047() functionjust passes "\n" through, without making it a
   folding whitespace followed by a newline, to help callers that want to
   use this function to produce a header line that is rfc 2822 conformant;

 - The patch introduces a new function add_rfc2047_multiline() that splits
   its input and performs line folding for such a caller (namely, the
   pp_title_line() function);

 - Another caller of add_rfc2047(), pp_user_info, is not changed, and it
   won't fold the name of the user that appear on the From: line.

It is unclear if the last point is really the right thing to do, though.
It is not a new problem that an author name that has a "\n" in it would
break the output, but we probably would want to fix that case too here?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help