pager: disable colors for some known-bad configurationsjk/color-for-more-pagers

Some pagers do not like the ANSI colors we print. It used to
be that one had to opt into the colors, and you would
usually check your pager config at the same time. These
days, we turn on color.ui by default, meaning that some
users may see broken "git log" from the start, before they
have configured anything.  They can fix it by turning off
"color.pager", but we should try to make things work
out-of-the-box as much as possible.

The common cause of this is that the user is using "less" or
"more", has setup its config variable in the environment (so
that we do not override it), but has not included "R" in
their config.

For other pagers, we simply continue to guess that the pager
can handle it. This is compatible with the current behavior
(and will keep exotic things like "diff-highlight | less"
working without further config). The downside is that other
random pagers may break.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

5 files changed, 50 insertions, 4 deletions

diff --git a/cache.h b/cache.h
index 83a27269b8..7fd19777ba 100644
--- a/cache.h
+++ b/cache.h
@@ -1215,7 +1215,8 @@ static inline ssize_t write_str_in_full(int fd, const char *str)
 extern void setup_pager(void);
 extern const char *pager_program;
 extern int pager_in_use(void);
-extern int pager_use_color;
+extern int pager_use_color_config;
+extern int pager_use_color(void);
 extern int term_columns(void);
 extern int decimal_width(int);
 extern int check_pager_config(const char *cmd);
diff --git a/color.c b/color.c
index f672885b71..ffbff40533 100644
--- a/color.c
+++ b/color.c
@@ -184,7 +184,7 @@ static int check_auto_color(void)
 {
 	if (color_stdout_is_tty < 0)
 		color_stdout_is_tty = isatty(1);
-	if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
+	if (color_stdout_is_tty || (pager_in_use() && pager_use_color())) {
 		char *term = getenv("TERM");
 		if (term && strcmp(term, "dumb"))
 			return 1;
diff --git a/config.c b/config.c
index d969a5aefc..2a8236bd8f 100644
--- a/config.c
+++ b/config.c
@@ -991,7 +991,7 @@ int git_default_config(const char *var, const char *value, void *dummy)
 		return git_default_advice_config(var, value);
 
 	if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) {
-		pager_use_color = git_config_bool(var,value);
+		pager_use_color_config = git_config_bool(var,value);
 		return 0;
 	}
 
diff --git a/environment.c b/environment.c
index 3c76905b9f..5cd642fcb3 100644
--- a/environment.c
+++ b/environment.c
@@ -39,7 +39,7 @@ size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
 size_t delta_base_cache_limit = 16 * 1024 * 1024;
 unsigned long big_file_threshold = 512 * 1024 * 1024;
 const char *pager_program;
-int pager_use_color = 1;
+int pager_use_color_config = -1;
 const char *editor_program;
 const char *askpass_program;
 const char *excludes_file;
diff --git a/pager.c b/pager.c
index 2303164f4e..63e9252faf 100644
--- a/pager.c
+++ b/pager.c
@@ -184,3 +184,48 @@ int check_pager_config(const char *cmd)
 		pager_program = c.value;
 	return c.want;
 }
+
+static int pager_can_handle_color(void)
+{
+	const char *pager = git_pager(1);
+
+	/*
+	 * If it's less, we automatically set "R" and can handle color,
+	 * unless the user already has a "LESS" variable that does not
+	 * include "R".
+	 */
+	if (!strcmp(pager, "less")) {
+		const char *x = getenv("LESS");
+		return !x || !!strchr(x, 'R');
+	}
+
+	if (!strcmp(pager, "more")) {
+#ifdef PAGER_MORE_UNDERSTANDS_R
+		/*
+		 * An advanced "more" that knows "R" is in the same boat as
+		 * "less".
+		 */
+		const char *x = getenv("MORE");
+		return !x || !!strchr(x, 'R');
+#else
+		/*
+		 * For a more primitive "more", just assume that it will pass
+		 * through the control codes verbatim.
+		 */
+		return 1;
+#endif
+	}
+
+	/*
+	 * Otherwise, we don't recognize it. Guess that it can probably handle
+	 * color. This matches what we have done historically.
+	 */
+	return 1;
+}
+
+int pager_use_color(void)
+{
+	if (pager_use_color_config < 0)
+		pager_use_color_config = pager_can_handle_color();
+	return pager_use_color_config;
+}