Add comment lines to patch format

Hi,

This is just an idea to help spot subtle changes in patch form (e.g. visible over email). A line starting with '=' (or something else) is treated like a comment line and ignored by "git apply". We could use this line to mark special things in the previous line.

One thing is trailing space, like demonstrated in the patch below, because trailing spaces may be intentional sometimes. But I'd like to incorporate some word-diff goodness in patch format using this comment line to spot few/single character addition/removal.

For example, instead of showing

- "this is a string"
+ N_("this is a string")

we could show

-  "this is a  string"
=  ..... .. . ^.......
+  N_("this is a string")
=  ^^^..... .. . .......^

If anyone knows a tool with similar feature, I'd greatly appreciate it (as the Internet taught me, everything I think of is already thought of/implemented by someone)

-- 8< --

index 77edd50..09d8c19 100644
@@ -1110,6 +1110,30 @@ static void find_lno(const char *line, struct emit_callback *ecbdata)
 	ecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);
 }
 
+static void annotate_line(struct emit_callback *ecbdata,
+			  const char *line, unsigned long len)
+{
+	char *buf, *s;
+	int trailing = 1;
+	if (!isspace(line[len-2]))   

= .. .......................^^^
+ return;
+ buf = malloc(len);
+ memcpy(buf, line, len);
+ buf[0] = '=';
+ for (s = buf + len - 2; s > buf; s--) {
+ if (trailing && isspace(*s))
+ *s = '^';
+ else
+ trailing = 0;
+ if (!trailing && !isspace(*s))
+ *s = '.';
+ }
+ emit_line(ecbdata->opt,
+ diff_get_color(ecbdata->color_diff, DIFF_PLAIN),
+ diff_get_color(ecbdata->color_diff, DIFF_RESET), buf, len);
+ free(buf);
+}
+
static void fn_out_consume(void *priv, char *line, unsigned long len)
{
struct emit_callback *ecbdata = priv;
@@ -1208,17 +1232,26 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
return;
}

- if (line[0] != '+') {
- const char *color =
- diff_get_color(ecbdata->color_diff,
- line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
+ switch (line[0]) {
+ case '-':
ecbdata->lno_in_preimage++;
- if (line[0] == ' ')
- ecbdata->lno_in_postimage++;
- emit_line(ecbdata->opt, color, reset, line, len);
- } else {
+ emit_line(ecbdata->opt,
+ diff_get_color(ecbdata->color_diff, DIFF_FILE_OLD),
+ reset, line, len);
+ annotate_line(ecbdata, line, len);
+ break;
+ case '+':
ecbdata->lno_in_postimage++; emit_add_line(reset, ecbdata, line + 1, len - 1);
+ annotate_line(ecbdata, line, len);
+ break;
+ case ' ':
+ ecbdata->lno_in_preimage++;
+ ecbdata->lno_in_postimage++;
+ emit_line(ecbdata->opt, plain, reset, line, len);
+ break;
+ default:
+ die("huh? a diff line starting with '%c'??", line[0]);
}
}

-- 8< --

Duy

-- 
Duy

Look in the archive and I think you will find a patch by me that implemented this not with '=' but with some other character, possibly with a matching patch to 'apply' without which its output is not usable. It was back when I was still a contributor, I think, so I do not remember the details.

Nothing stands out from gmane interface up until you became maintainer. But I might overlook because you exceeded 1000 mails just in three months. That might be Linus' secret criteria for a new
maintainer: pick the first one that passes 1000 mails limit ;)

Anyway don't waste your time looking back in your mail archive.

Duy

Have you looked at contrib/diff-highlight?

I think your approach is interesting because it can annotate much more than just "this part is interesting". Sometimes when I send a patch, I'd like to be able to comment in-line, like:

+ if (bar > 3)
+ foo(bar);
= Yeah, this magic "3" is ugly, and we should handle it = through $whatever in the re-roll.
+ else
+ something_else();

I usually just write something in the cover letter, or reply to myself with comments inline. I don't know how much I would use it in practice, but it might be a neat thing for "git apply" to simply ignore comment lines in the middle of a hunk.

Of course, this feature might not be worth breaking compatibility with every existing version of "git apply" and "patch" out there.

-Peff

I didn't. Its output looks too packed though.

Yeah. That's a good thing about in-patch comments.

One thing that's still bugging me is that we don't have a way to send machine-readable conflict resolutions in text form. But that's probably something else.

It's opt-in. And it's quite easy to convert back. A simple "sed -i '/^=/d'" would do.

Duy