Re: [PATCH] Bugfix: grep: Do not colorize output when -O is set

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 6, 2010, 19:38 UTC
Message-ID: <20100706193845.GA7438@burratino>
In-Reply-To: <4C2E1185.1040406@lsrfire.ath.cx>

Hi,

René Scharfe wrote:

> Hmm, but with --open-files-in-pager without argument or -Oless colours
> may be handled correctly and desirable.

Sorry I missed this before. Is there really a pager that will accept \e[36m as a command-line argument and do something reasonable with it?

> Turning colouring off with -O
> is probably the most sensible default, but is it possible to allow
> turning it back on explicitly (--color -O)?

A person trying that might be wanting to highlight matches in the pager rather than in argv itself. :) Unfortunately, it is not completely obvious how to comply.

‘less’ already highlights matches by default, though not in the color configured for git. grep -O will tell ‘less’ what to look for if there was just one pattern.

editors like vim tend to use syntax highlighting in addition to optionally highlighting search matches.

Probably a better solution is to recommend -C option, possibly implementing -C infinity so people don’t have to use -C 1000000.

But your point is well taken that the current behavior is confusing. How about the following?

diff --git a/builtin/grep.c b/builtin/grep.c
index 7a9427d..921f554 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -835,6 +835,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct string_list path_list = { NULL, 0, 0, 0 };
 	int i;
 	int dummy;
+	int use_color = -1;
 	int nongit = 0, use_index = 1;
 	struct option options[] = {
 		OPT_BOOLEAN(0, "cached", &cached,
@@ -881,7 +882,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			"print NUL after filenames"),
 		OPT_BOOLEAN('c', "count", &opt.count,
 			"show the number of matches instead of matching lines"),
-		OPT__COLOR(&opt.color, "highlight matches"),
+		OPT__COLOR(&use_color, "highlight matches"),
 		OPT_GROUP(""),
 		OPT_CALLBACK('C', NULL, &opt, "n",
 			"show <n> context lines before and after matches",
@@ -994,6 +995,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		argc--;
 	}
 
+	if (use_color != -1)
+		opt.color = use_color;
+
 	if (show_in_pager == default_pager)
 		show_in_pager = git_pager(1);
 	if (show_in_pager) {
@@ -1006,6 +1010,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		use_threads = 0;
 	}
 
+	if (show_in_pager && use_color)
+		die("cannot mix -O and --color");
 	if (!opt.pattern_list)
 		die("no pattern given.");
 	if (!opt.fixed && opt.ignore_case)