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

[Jonathan Nieder <jrnieder@gmail.com>]

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)