git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 094/104] diff: fix coloring of extended diff headers

From
Sam Vilain <sam.vilain@catalyst.net.nz>
Date
May 26, 2010, 06:01 UTC
Message-ID
<1274853674-18521-94-git-send-email-sam.vilain@catalyst.net.nz>
In-Reply-To
<1274853674-18521-1-git-send-email-sam.vilain@catalyst.net.nz>
From: Bert Wesarg <bert.wesarg@googlemail.com>

Coloring the extended headers where done as a whole not per line. less with option -R (which is the default from git) does not support this coloring mode because of performance reasons. The -r option would be an alternative but has problems with lines that are longer than the screen. Therefore stick to the idiom to color each line separately. The problem is, that the result of ill_metainfo() will also be used as an parameter to an external diff driver, so we need to disable coloring in this case.

Because coloring is now done inside fill_metainfo() we can simply add this string to the diff header and therefore keep the last newline in the extended header. This results also into the fact that the external diff driver now gets this last newline too. Which is a change in behavior but a good one.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c |   61 +++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/diff.c b/diff.c
index d0ecbc3..7e508dd 100644
--- a/diff.c
+++ b/diff.c
@@ -1650,21 +1650,21 @@ static void builtin_diff(const char *name_a,
 	if (lbl[0][0] == '/') {
 		/* /dev/null */
 		strbuf_addf(&header, "%snew file mode %06o%s\n", set, two->mode, reset);
-		if (xfrm_msg && xfrm_msg[0])
-			strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset);
+		if (xfrm_msg)
+			strbuf_addstr(&header, xfrm_msg);
 	}
 	else if (lbl[1][0] == '/') {
 		strbuf_addf(&header, "%sdeleted file mode %06o%s\n", set, one->mode, reset);
-		if (xfrm_msg && xfrm_msg[0])
-			strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset);
+		if (xfrm_msg)
+			strbuf_addstr(&header, xfrm_msg);
 	}
 	else {
 		if (one->mode != two->mode) {
 			strbuf_addf(&header, "%sold mode %06o%s\n", set, one->mode, reset);
 			strbuf_addf(&header, "%snew mode %06o%s\n", set, two->mode, reset);
 		}
-		if (xfrm_msg && xfrm_msg[0])
-			strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset);
+		if (xfrm_msg)
+			strbuf_addstr(&header, xfrm_msg);
 
 		/*
 		 * we do not run diff between different kind
@@ -2323,30 +2323,36 @@ static void fill_metainfo(struct strbuf *msg,
 			  struct diff_filespec *one,
 			  struct diff_filespec *two,
 			  struct diff_options *o,
-			  struct diff_filepair *p)
+			  struct diff_filepair *p,
+			  int use_color)
 {
+	const char *set = diff_get_color(use_color, DIFF_METAINFO);
+	const char *reset = diff_get_color(use_color, DIFF_RESET);
+
 	strbuf_init(msg, PATH_MAX * 2 + 300);
 	switch (p->status) {
 	case DIFF_STATUS_COPIED:
-		strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
-		strbuf_addstr(msg, "\ncopy from ");
+		strbuf_addf(msg, "%ssimilarity index %d%%",
+			    set, similarity_index(p));
+		strbuf_addf(msg, "%s\n%scopy from ", reset, set);
 		quote_c_style(name, msg, NULL, 0);
-		strbuf_addstr(msg, "\ncopy to ");
+		strbuf_addf(msg, "%s\n%scopy to ", reset, set);
 		quote_c_style(other, msg, NULL, 0);
-		strbuf_addch(msg, '\n');
+		strbuf_addf(msg, "%s\n", reset);
 		break;
 	case DIFF_STATUS_RENAMED:
-		strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
-		strbuf_addstr(msg, "\nrename from ");
+		strbuf_addf(msg, "%ssimilarity index %d%%",
+			    set, similarity_index(p));
+		strbuf_addf(msg, "%s\n%srename from ", reset, set);
 		quote_c_style(name, msg, NULL, 0);
-		strbuf_addstr(msg, "\nrename to ");
+		strbuf_addf(msg, "%s\n%srename to ", reset, set);
 		quote_c_style(other, msg, NULL, 0);
-		strbuf_addch(msg, '\n');
+		strbuf_addf(msg, "%s\n", reset);
 		break;
 	case DIFF_STATUS_MODIFIED:
 		if (p->score) {
-			strbuf_addf(msg, "dissimilarity index %d%%\n",
-				    similarity_index(p));
+			strbuf_addf(msg, "%sdissimilarity index %d%%%s\n",
+				    set, similarity_index(p), reset);
 			break;
 		}
 		/* fallthru */
@@ -2363,15 +2369,13 @@ static void fill_metainfo(struct strbuf *msg,
 			    (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
 				abbrev = 40;
 		}
-		strbuf_addf(msg, "index %.*s..%.*s",
+		strbuf_addf(msg, "%sindex %.*s..%.*s", set,
 			    abbrev, sha1_to_hex(one->sha1),
 			    abbrev, sha1_to_hex(two->sha1));
 		if (one->mode == two->mode)
 			strbuf_addf(msg, " %06o", one->mode);
-		strbuf_addch(msg, '\n');
+		strbuf_addf(msg, "%s\n", reset);
 	}
-	if (msg->len)
-		strbuf_setlen(msg, msg->len - 1);
 }
 
 static void run_diff_cmd(const char *pgm,
@@ -2387,11 +2391,6 @@ static void run_diff_cmd(const char *pgm,
 	const char *xfrm_msg = NULL;
 	int complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;
 
-	if (msg) {
-		fill_metainfo(msg, name, other, one, two, o, p);
-		xfrm_msg = msg->len ? msg->buf : NULL;
-	}
-
 	if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
 		pgm = NULL;
 	else {
@@ -2400,6 +2399,16 @@ static void run_diff_cmd(const char *pgm,
 			pgm = drv->external;
 	}
 
+	if (msg) {
+		/*
+		 * don't use colors when the header is intended for an
+		 * external diff driver
+		 */
+		fill_metainfo(msg, name, other, one, two, o, p,
+			      DIFF_OPT_TST(o, COLOR_DIFF) && !pgm);
+		xfrm_msg = msg->len ? msg->buf : NULL;
+	}
+
 	if (pgm) {
 		run_external_diff(pgm, name, other, one, two, xfrm_msg,
 				  complete_rewrite);

-- 
1.7.1.rc2.333.gb2668
Previous: Sam VilainNext: Sam Vilain
Message 12 of 23 in “tests: chmod +x t5150”
  1. 075/104 tests: chmod +x t5150Sam Vilain, May 26, 2010
  2. 076/104 t7604-merge-custom-message: shift expected output creationSam Vilain, May 26, 2010
  3. 082/104 fmt-merge-msg: add function to append shortlog onlySam Vilain, May 26, 2010
  4. 084/104 autocrlf: Make it work also for un-normalized repositoriesSam Vilain, May 26, 2010
  5. 086/104 gitweb: Use @diff_opts while using format-patchSam Vilain, May 26, 2010
  6. 087/104 hash_object: correction for zero length fileSam Vilain, May 26, 2010
  7. 088/104 for-each-ref: Field with abbreviated objectnameSam Vilain, May 26, 2010
  8. 090/104 Documentation: rebase -i ignores options passed to "git am"Sam Vilain, May 26, 2010
  9. 091/104 Documentation: fix minor inconsistencySam Vilain, May 26, 2010
  10. 092/104 Documentation/gitdiffcore: fix order in pickaxe descriptionSam Vilain, May 26, 2010
  11. 093/104 post-receive-email: document command-line modeSam Vilain, May 26, 2010
  12. 094/104 diff: fix coloring of extended diff headersSam Vilain, May 26, 2010
  13. 095/104 Fix "Out of memory? mmap failed" for files larger than 4GB on WindowsSam Vilain, May 26, 2010
  14. 096/104 start_command: close cmd->err descriptor when fork/spawn failsSam Vilain, May 26, 2010
  15. 097/104 Fix checkout of large files to network shares on Windows XPSam Vilain, May 26, 2010
  16. 098/104 mingw: use _commit to implement fsyncSam Vilain, May 26, 2010
  17. 099/104 Recent MinGW has a C99 implementation of snprintf functionsSam Vilain, May 26, 2010
  18. 100/104 Complete prototype of git_config_from_parameters()Sam Vilain, May 26, 2010
  19. 101/104 test get_git_work_tree() return value for NULLSam Vilain, May 26, 2010
  20. 102/104 t7502-commit: fix spellingSam Vilain, May 26, 2010
  21. 103/104 show-branch: use DEFAULT_ABBREV instead of 7Sam Vilain, May 26, 2010
  22. 104/104 Documentation/SubmittingPatches: clarify GMail section and SMTPSam Vilain, May 26, 2010
  23. Sverre RabbelierMay 26, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.