From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff King Subject: [PATCH 3/3] pager: disable colors for some known-bad configurations Date: Thu, 16 Jan 2014 23:24:50 -0500 Message-ID: <20140117042450.GC23443@sigill.intra.peff.net> References: <20140117041430.GB19551@sigill.intra.peff.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: Jonathan Nieder , Yuri To: git@vger.kernel.org X-From: git-owner@vger.kernel.org Fri Jan 17 05:24:56 2014 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1W40zI-0004AL-3u for gcvg-git-2@plane.gmane.org; Fri, 17 Jan 2014 05:24:56 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751790AbaAQEYx (ORCPT ); Thu, 16 Jan 2014 23:24:53 -0500 Received: from cloud.peff.net ([50.56.180.127]:33992 "HELO peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751369AbaAQEYv (ORCPT ); Thu, 16 Jan 2014 23:24:51 -0500 Received: (qmail 15352 invoked by uid 102); 17 Jan 2014 04:24:51 -0000 Received: from c-71-63-4-13.hsd1.va.comcast.net (HELO sigill.intra.peff.net) (71.63.4.13) (smtp-auth username relayok, mechanism cram-md5) by peff.net (qpsmtpd/0.84) with ESMTPA; Thu, 16 Jan 2014 22:24:51 -0600 Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Thu, 16 Jan 2014 23:24:50 -0500 Content-Disposition: inline In-Reply-To: <20140117041430.GB19551@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: 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 --- 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