git am and the wrong chunk of ---

DISCUSSION H. Peter Anvin 4 participants 8 messages Jun 15, 2016

Share & tools

H. Peter Anvin

author

2016-06-15 22:54 UTC #0 lore raw

Hello,

I have some contributors who consistently put their commentary *before* the "---" line rather than *after* it, presumably with the notion that it is some kind of "cover text". This messes with "git am", and so I end up having to edit those posts manually.

I have tried git am --scissors and it doesn't seem to solve the problem.

Is there any other option which can be used to automatically process such a patch?

-hpa

Signature · 2 lines

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Jeff King

2016-06-15 22:54 UTC #1 lore raw

Quoted 9 lines · H. Peter Anvin

I have some contributors who consistently put their commentary *before* the "---" line rather than *after* it, presumably with the notion that it is some kind of "cover text". This messes with "git am", and so I end up having to edit those posts manually.
I have tried git am --scissors and it doesn't seem to solve the problem.
Is there any other option which can be used to automatically process such a patch?

If I understand your issue, somebody is writing:

From: them
To: you
Date: ...
Subject: [PATCH] subject line

commit message body
....

some cover letter material that should go below the "---"
---
[diffstat + diff]

How do you know when the commit message body ends, and the cover letter begins? We already have two machine-readable formats for separating the two ("---" after the commit message, and "-- >8 --" scissors before). Is there some machine-readable hint? Is it always the paragraph before the "---"? Chopping that off unconditionally seems like a dangerous heuristic.

-Peff

Andreas Ericsson

2016-06-15 22:54 UTC #2 lore raw

Quoted 28 lines · Jeff King

Quoted 9 lines · H. Peter Anvin
I have some contributors who consistently put their commentary *before* the "---" line rather than *after* it, presumably with the notion that it is some kind of "cover text". This messes with "git am", and so I end up having to edit those posts manually.
I have tried git am --scissors and it doesn't seem to solve the problem.
Is there any other option which can be used to automatically process such a patch?
If I understand your issue, somebody is writing:
From: them
To: you
Date: ...
Subject: [PATCH] subject line
commit message body
....
some cover letter material that should go below the "---"
---
[diffstat + diff]

Quoted 7 lines

How do you know when the commit message body ends, and the cover letter begins? We already have two machine-readable formats for separating the two ("---" after the commit message, and "-- >8 --" scissors before). Is there some machine-readable hint? Is it always the paragraph before the "---"? Chopping that off unconditionally seems like a dangerous heuristic.

End of SOB lines might be a good cutoff, if they're present. I've never seen anyone put commit message text below them anyway.

Signature · 7 lines

Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

Junio C Hamano

2016-06-15 22:54 UTC #3 lore raw

In reply to Jeff King

Quoted 15 lines · Jeff King

If I understand your issue, somebody is writing:
From: them
To: you
Date: ...
Subject: [PATCH] subject line
commit message body
....
some cover letter material that should go below the "---"
---
[diffstat + diff]

Quoted 6 lines

How do you know when the commit message body ends, and the cover letter begins? We already have two machine-readable formats for separating the two ("---" after the commit message, and "-- >8 --" scissors before). Is there some machine-readable hint? Is it always the paragraph before the "---"? Chopping that off unconditionally seems like a dangerous heuristic.

Or it could be like this:

...
Subject: [PATCH] patch title

Heya,

I was walking my dog when I found a solution to this problem the other day. Here it is.

commit message body

S-o-b: ...
---

And I agree that clever heuristics are dangerous. We need to draw a line somewhere anyway, and the line should be at the place that is easily understandable to people. That means mechanically parseable and easy to follow convention to use markers e.g. "---".

H. Peter Anvin

author

2016-06-15 22:54 UTC #4 lore raw

The users I am referring to generally have a --- line, rather than a scissor, between the cover text and commit. Also, there is (almost) always a From: line and subject at the top of the patch proper.

Quoted 47 lines · Junio C Hamano

Quoted 8 lines · Jeff King
If I understand your issue, somebody is writing:
From: them
To: you
Date: ...
Subject: [PATCH] subject line
Quoted 8 lines
commit message body
....
some cover letter material that should go below the "---"
---
[diffstat + diff]
How do you know when the commit message body ends, and the cover
letter
Quoted 1 line
begins? We already have two machine-readable formats for separating
the
Quoted 1 line
two ("---" after the commit message, and "-- >8 --" scissors before).
Is
Quoted 1 line
there some machine-readable hint? Is it always the paragraph before
the
Quoted 2 lines
"---"? Chopping that off unconditionally seems like a dangerous heuristic.
Or it could be like this:
...
Subject: [PATCH] patch title
Heya,
I was walking my dog when I found a solution to this problem the other day. Here it is.
   commit message body

   S-o-b: ...
   ---
And I agree that clever heuristics are dangerous. We need to draw a line somewhere anyway, and the line should be at the place that is easily understandable to people. That means mechanically parseable and easy to follow convention to use markers e.g. "---".

Signature · 1 line

Sent from my mobile phone. Please excuse brevity and lack of formatting.

Junio C Hamano

2016-06-15 22:54 UTC #5 lore raw

Quoted 4 lines · H. Peter Anvin

The users I am referring to generally have a --- line, rather than a scissor, between the cover text and commit. Also, there is (almost) always a From: line and subject at the top of the patch proper.

Oh, so it is more like this?

From: author name <[email protected]>
Date: author date
Subject: patch title

Heya,

I was walking my dog when I found a solution to this problem the other day. Here it is.

---
>From 755e8b3f35e3991a735a6be740eda4567d45a741 Mon Sep 17 00:00:00 2001
From: author name <[email protected]>
Date: random date we do not care
Subject: patch title

commit message body

---

We could teach "am -c" to recognize the format-patch file magic

    "^[>]From [0-9a-f]{40} Mon Sep 17 00:00:00 2001"

as another form of accepted scissors, I guess.

Something like the attached (untested) patch, perhaps.

But I am fairly negative on it.

Where would it end? After all, the top "---" is not something our tools are generating, but is manually typed by the users.

I do not think it is unreasonable to expect that they are capable and intelligent enough to guess that "---" is _not_ the way to say "cut here and what follows are the log message", when "---" is already the way to say "cut here, and what we saw up to this point is the log message".

 builtin/mailinfo.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

builtin/mailinfo.c +17 -1

index eaf9e15..62ea09d 100644
@@ -730,6 +730,22 @@ static inline int patchbreak(const struct strbuf *line)
 	return 0;
 }
 
+static int is_format_patch_magic(const struct strbuf *line)
+{
+	const char *buf = line->buf;
+	size_t len = line->len;
+
+	if (len && *buf == '>') {
+		buf++;
+		len--;
+	}
+	if (len < 70)
+		return 0;
+	return (!memcmp(buf, "From ", 5) &&
+		strspn(buf + 5, "0123456789abcdef") == 40 &&
+		!memcmp(buf + 46, "Mon Sep 17 00:00:00 2001", 24));
+}
+
 static int is_scissors_line(const struct strbuf *line)
 {
 	size_t i, len = line->len;

@@ -807,7 +823,7 @@ static int handle_commit_msg(struct strbuf *line)
 	if (metainfo_charset)
 		convert_to_utf8(line, charset.buf);
 
-	if (use_scissors && is_scissors_line(line)) {
+	if (use_scissors && (is_scissors_line(line) || is_format_patch_magic(line))) {
 		int i;
 		if (fseek(cmitmsg, 0L, SEEK_SET))
 			die_errno("Could not rewind output message file");

H. Peter Anvin

author

2016-06-15 22:54 UTC #6 lore raw

Quoted 24 lines · Junio C Hamano

Quoted 4 lines · H. Peter Anvin
The users I am referring to generally have a --- line, rather than a scissor, between the cover text and commit. Also, there is (almost) always a From: line and subject at the top of the patch proper.
Oh, so it is more like this?
From: author name <[email protected]>
Date: author date
Subject: patch title
Heya,
I was walking my dog when I found a solution to this problem the other day. Here it is.
---
>From 755e8b3f35e3991a735a6be740eda4567d45a741 Mon Sep 17 00:00:00 2001
From: author name <[email protected]>
Date: random date we do not care
Subject: patch title

Quoted 4 lines

commit message body
---

That is exactly what I see, except usually with the mbox header.

However, it makes sense to me to treat From: as a scissor (we can then ignore the preceding --- completely).

-hpa

Junio C Hamano

2016-06-15 22:54 UTC #7 lore raw

In reply to H. Peter Anvin

Quoted 11 lines · H. Peter Anvin

Hello,
I have some contributors who consistently put their commentary *before* the "---" line rather than *after* it, presumably with the notion that it is some kind of "cover text". This messes with "git am", and so I end up having to edit those posts manually.
I have tried git am --scissors and it doesn't seem to solve the problem.
Is there any other option which can be used to automatically process such a patch?

I hate to be the one who is telling you this, but if the submitter cannot be trained to write supporting material after "---" as the convention across git using projects suggest him to do, it is likely that he didn't write supporting material before the scissors, or did not resist the temptation to deviate from the accepted shape of the scissors (e.g. "-- >8 --") just to be creative. For that matter, I would be mildly surprised if the material in the middle is usable as is as an acceptable log message from such a submitter X-<.

So in short, no, --scissors (or -c in short) is not any more magical than the traditional "---".