[PATCH 3/3] pager: disable colors for some known-bad configurations
- From
Jeff King <peff@peff.net>
- Date
- Jan 17, 2014, 04:24 UTC
- Message-ID
- <20140117042450.GC23443@sigill.intra.peff.net>
- In-Reply-To
- <20140117041430.GB19551@sigill.intra.peff.net>
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> --- cache.h | 3 ++- color.c | 2 +- config.c | 2 +- environment.c | 2 +- pager.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h index 83a2726..7fd1977 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 f672885..ffbff40 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 d969a5a..2a8236b 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 3c76905..5cd642f 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 2303164..63e9252 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; +}
-- 1.8.5.2.500.g8060133