Re: [PATCH] diff-format.txt: Combined diff...

Re: [PATCH] diff-format.txt: Combined diff format documentation supplement

From: Junio C Hamano <hidden>
Date: 2016-08-11 19:35:28

Jakub Narebski [off-list ref] writes:

So here you have. It should perhaps get review on validity by someone
well versed in the combined diff generation code. There are some guesses
here...

Thanks.

I guess review by the original author would be good enough;
this is entirely my code -- it was done while Linus and gang
was having fun in NZ, if I recall correctly ;-).

It compiles, but the output was not inspected.

I've done minimal asciidoc mark-up fixes.  Troff man output look
horrible but that is not limited to this man page -- it looks
quite wrong whenever numbered list with displayed examples are
used.

Re: [PATCH] diff-format.txt: Combined diff format documentation supplement

From: Junio C Hamano <hidden>
Date: 2016-08-11 20:05:48

By the way, this is only minimally tested, but this patch fills
the blanks in your documentation.

The combine_diff_path structure _does_ keep track of the
original path in each parent, so renames _could_ be shown (we do
not keep the score, though), but the question is how.  "rename to"
would obviously be "the path in the merge result", but "rename from"
needs to be shown for all parents when we have rename from any
of the parents.  "rename from" line as in the usual one parent diff
does not have any place to say "which parent" (because there is
no need), so showing the usual "rename from" for only parents
that the result has rename from makes the output useless -- we
cannot tell "from which parent" from such an output.  I feel
that an evil merge is rare enough that worrying about showing
rename line is probably not worth the effort.

-- >8 --
[PATCH] combine-diff: a few more finishing touches.

"new file" and "deleted file" were already reported in the
original code, but the logic was not as transparent as it could
have.  This uses a few variables and more comments to clarify
the flow.  The rule is: (1) if a path exists in the merge result
when no parent had it, we report "new" (otherwise it came from
the parents, as opposed to have added by the evil merge). (2) if
the path does not exist in the merge result, it is "deleted".

Since we can say "new" and "deleted", there is no reason not to
follow the /dev/null convention.  This fixes it.

Appending function name after @@@ ... @@@ is trivial, so
implement it.

Signed-off-by: Junio C Hamano <redacted>
---
 combine-diff.c |   48 +++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 46d9121..01a8437 100644
--- a/combine-diff.c
+++ b/combine-diff.c

@@ -489,6 +489,12 @@ static void show_parent_lno(struct sline
 	printf(" -%lu,%lu", l0, l1-l0);
 }
 
+static int hunk_comment_line(const char *bol)
+{
+	int ch = *bol & 0xff;
+	return (isalpha(ch) || ch == '_' || ch == '$');
+}
+
 static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
 		       int use_color)
 {

@@ -508,8 +514,13 @@ static void dump_sline(struct sline *sli
 		struct sline *sl = &sline[lno];
 		unsigned long hunk_end;
 		unsigned long rlines;
-		while (lno <= cnt && !(sline[lno].flag & mark))
+		const char *hunk_comment = NULL;
+
+		while (lno <= cnt && !(sline[lno].flag & mark)) {
+			if (hunk_comment_line(sline[lno].bol))
+				hunk_comment = sline[lno].bol;
 			lno++;
+		}
 		if (cnt < lno)
 			break;
 		else {

@@ -526,6 +537,22 @@ static void dump_sline(struct sline *sli
 			show_parent_lno(sline, lno, hunk_end, i);
 		printf(" +%lu,%lu ", lno+1, rlines);
 		for (i = 0; i <= num_parent; i++) putchar(combine_marker);
+
+		if (hunk_comment) {
+			int comment_end = 0;
+			for (i = 0; i < 40; i++) {
+				int ch = hunk_comment[i] & 0xff;
+				if (!ch || ch == '\n')
+					break;
+				if (!isspace(ch))
+				    comment_end = i;
+			}
+			if (comment_end)
+				putchar(' ');
+			for (i = 0; i < comment_end; i++)
+				putchar(hunk_comment[i]);
+		}
+
 		printf("%s\n", c_reset);
 		while (lno < hunk_end) {
 			struct lline *ll;

@@ -707,6 +734,8 @@ static void show_patch_diff(struct combi
 		int use_color = opt->color_diff;
 		const char *c_meta = diff_get_color(use_color, DIFF_METAINFO);
 		const char *c_reset = diff_get_color(use_color, DIFF_RESET);
+		int added = 0;
+		int deleted = 0;
 
 		if (rev->loginfo)
 			show_log(rev, opt->msg_sep);

@@ -722,7 +751,10 @@ static void show_patch_diff(struct combi
 		printf("..%s%s\n", abb, c_reset);
 
 		if (mode_differs) {
-			int added = !!elem->mode;
+			deleted = !elem->mode;
+
+			/* We say it was added if nobody had it */
+			added = !deleted;
 			for (i = 0; added && i < num_parent; i++)
 				if (elem->parent[i].status !=
 				    DIFF_STATUS_ADDED)

@@ -731,7 +763,7 @@ static void show_patch_diff(struct combi
 				printf("%snew file mode %06o",
 				       c_meta, elem->mode);
 			else {
-				if (!elem->mode)
+				if (deleted)
 					printf("%sdeleted file ", c_meta);
 				printf("mode ");
 				for (i = 0; i < num_parent; i++) {

@@ -743,8 +775,14 @@ static void show_patch_diff(struct combi
 			}
 			printf("%s\n", c_reset);
 		}
-		dump_quoted_path("--- a/", elem->path, c_meta, c_reset);
-		dump_quoted_path("+++ b/", elem->path, c_meta, c_reset);
+		if (added)
+			dump_quoted_path("--- /dev/", "null", c_meta, c_reset);
+		else
+			dump_quoted_path("--- a/", elem->path, c_meta, c_reset);
+		if (deleted)
+			dump_quoted_path("+++ /dev/", "null", c_meta, c_reset);
+		else
+			dump_quoted_path("+++ b/", elem->path, c_meta, c_reset);
 		dump_sline(sline, cnt, num_parent, opt->color_diff);
 	}
 	free(result);

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help