Re: [PATCH] pager: disable color when pager is "more"
Junio C Hamano <gits...@pobox.com> wrote: > Jeff King <p...@peff.net> writes: > > On Mon, Jul 18, 2016 at 09:19:07AM +, Eric Wong wrote: > >> Johannes Schindelin <johannes.schinde...@gmx.de> wrote: > >> > On Sun, 17 Jul 2016, n...@dad.org wrote: > >> > > 'git diff' outputs escape characters which clutter my terminal. Yes, I > >> > > can sed them out, but then why are they there? > >> > > >> > Those are most likely the ANSI sequences to add color. Can you call Git > >> > with the --no-color option and see whether the escape characters go away? > >> > >> Norm: do you have PAGER=more set by any chance? > >> Perhaps changing it to "less" will allow you to preserve colors. > >> > >> I saw a similar or identical problem during my vacation in > >> FreeBSD-land. Perhaps the out-of-the-box experience can be > >> improved: > >> > >> -8<- > >> Subject: [PATCH] pager: disable color when pager is "more" > > > > This is the tip of a smaller iceberg. See > > > > http://public-inbox.org/git/52D87A79.6060600%40rawbw.com/t/#u > > > > for more discussion, and some patches that fix more cases (like "LESS" > > without "R", or "more" that _does_ understand "R"). I think it was > > discarded as being a little too intimate with the details of pagers, but > > it does suck that the out-of-the-box experience on FreeBSD is not good. > > Maybe we should revisit it. Yes; I'd prefer not to get too intimate with the details of pagers, either, and I think we should err on the side of monochrome for systems we do not know much about. > Yup, the three-patch series at > > > http://public-inbox.org/git/20140117041430.GB19551%40sigill.intra.peff.net/ I am not a fan of adding #ifdefs for platform-specific things; so I prefer starting with my original patch to disable colors for "more". (or, even disable colors for everything which is not "less" or "lv") > would be a safe starting point that is low-impact. I think what > ended up being discarded was a more elaborate side topic that > started from exploring the possibility of checking if LESS has 'R' > in it to see if it is possible to help people with LESS that does > not allow coloring explicitly exported. Heh... (see below) > I do not think the approach in the same thread suggested by Kyle > > > http://public-inbox.org/git/62DB6DEF-8B39-4481-BA06-245BF45233E5%40gmail.com/ > > is too bad, either. I like Kyle's suggestion, and I think that can be a good transition from your original patch to move pager configuration into the build: https://public-inbox.org/git/xmqq61piw4yf@gitster.dls.corp.google.com/ I've updated just that and pushed just that to the "pager-build" topic of git://bogomips.org/git-svn So I'd prefer we drop the later automatic header generation changes that got squashed into later iterations. Unfortunately, it looks like that all got lost in Jeff's 13-patch "makefile refactoring" topic starting at: https://public-inbox.org/git/20140205174823.ga15...@sigill.intra.peff.net/ Yeah, we tend to get sidetracked :x -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pager: disable color when pager is "more"
On Mon, Jul 18, 2016 at 10:19:09AM -0700, Junio C Hamano wrote: > > This is the tip of a smaller iceberg. See > > > > http://public-inbox.org/git/52D87A79.6060600%40rawbw.com/t/#u > > > > for more discussion, and some patches that fix more cases (like "LESS" > > without "R", or "more" that _does_ understand "R"). I think it was > > discarded as being a little too intimate with the details of pagers, but > > it does suck that the out-of-the-box experience on FreeBSD is not good. > > Maybe we should revisit it. > > Yup, the three-patch series at > > > http://public-inbox.org/git/20140117041430.GB19551%40sigill.intra.peff.net/ > > would be a safe starting point that is low-impact. I think what > ended up being discarded was a more elaborate side topic that > started from exploring the possibility of checking if LESS has 'R' > in it to see if it is possible to help people with LESS that does > not allow coloring explicitly exported. Yeah, I only re-skimmed the thread, but I had the same impression. I am traveling the next few days, so I will probably not get to it soon. If anybody wants to pick up and rebase/polish those patches as necessary, be my guest. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pager: disable color when pager is "more"
Jeff King <p...@peff.net> writes: > On Mon, Jul 18, 2016 at 09:19:07AM +, Eric Wong wrote: > >> Johannes Schindelin <johannes.schinde...@gmx.de> wrote: >> > On Sun, 17 Jul 2016, n...@dad.org wrote: >> > > 'git diff' outputs escape characters which clutter my terminal. Yes, I >> > > can sed them out, but then why are they there? >> > >> > Those are most likely the ANSI sequences to add color. Can you call Git >> > with the --no-color option and see whether the escape characters go away? >> >> Norm: do you have PAGER=more set by any chance? >> Perhaps changing it to "less" will allow you to preserve colors. >> >> I saw a similar or identical problem during my vacation in >> FreeBSD-land. Perhaps the out-of-the-box experience can be >> improved: >> >> -8<- >> Subject: [PATCH] pager: disable color when pager is "more" > > This is the tip of a smaller iceberg. See > > http://public-inbox.org/git/52D87A79.6060600%40rawbw.com/t/#u > > for more discussion, and some patches that fix more cases (like "LESS" > without "R", or "more" that _does_ understand "R"). I think it was > discarded as being a little too intimate with the details of pagers, but > it does suck that the out-of-the-box experience on FreeBSD is not good. > Maybe we should revisit it. Yup, the three-patch series at http://public-inbox.org/git/20140117041430.GB19551%40sigill.intra.peff.net/ would be a safe starting point that is low-impact. I think what ended up being discarded was a more elaborate side topic that started from exploring the possibility of checking if LESS has 'R' in it to see if it is possible to help people with LESS that does not allow coloring explicitly exported. I do not think the approach in the same thread suggested by Kyle http://public-inbox.org/git/62DB6DEF-8B39-4481-BA06-245BF45233E5%40gmail.com/ is too bad, either. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pager: disable color when pager is "more"
On Mon, Jul 18, 2016 at 09:19:07AM +, Eric Wong wrote: > Johannes Schindelin <johannes.schinde...@gmx.de> wrote: > > On Sun, 17 Jul 2016, n...@dad.org wrote: > > > 'git diff' outputs escape characters which clutter my terminal. Yes, I > > > can sed them out, but then why are they there? > > > > Those are most likely the ANSI sequences to add color. Can you call Git > > with the --no-color option and see whether the escape characters go away? > > Norm: do you have PAGER=more set by any chance? > Perhaps changing it to "less" will allow you to preserve colors. > > I saw a similar or identical problem during my vacation in > FreeBSD-land. Perhaps the out-of-the-box experience can be > improved: > > -8<- > Subject: [PATCH] pager: disable color when pager is "more" This is the tip of a smaller iceberg. See http://public-inbox.org/git/52D87A79.6060600%40rawbw.com/t/#u for more discussion, and some patches that fix more cases (like "LESS" without "R", or "more" that _does_ understand "R"). I think it was discarded as being a little too intimate with the details of pagers, but it does suck that the out-of-the-box experience on FreeBSD is not good. Maybe we should revisit it. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pager: disable color when pager is "more"
Johannes Schindelin <johannes.schinde...@gmx.de> wrote:
> On Sun, 17 Jul 2016, n...@dad.org wrote:
> > 'git diff' outputs escape characters which clutter my terminal. Yes, I
> > can sed them out, but then why are they there?
>
> Those are most likely the ANSI sequences to add color. Can you call Git
> with the --no-color option and see whether the escape characters go away?
Norm: do you have PAGER=more set by any chance?
Perhaps changing it to "less" will allow you to preserve colors.
I saw a similar or identical problem during my vacation in
FreeBSD-land. Perhaps the out-of-the-box experience can be
improved:
-8<-
Subject: [PATCH] pager: disable color when pager is "more"
more(1) traditionally cannot handle colors.
On FreeBSD 10.3, a new user ~/.profile explicitly sets
PAGER=more, but does not configure it to display colors, leading
to a bad out-of-the-box experience with escape sequences being
seen by the user.
In the FreeBSD 10.3 case, /usr/bin/more is actually a hardlink
to /usr/bin/less and capable of handling colors. While we could
set MORE=FRX, this breaks other more(1) implementations,
including the one provided by util-linux on common GNU/Linux
systems.
So take the safe route and assume anybody still using more(1)
today can live with monochrome output, but acknowledge 'R'
in the MORE environment variable if it was set by the user.
Signed-off-by: Eric Wong <e...@80x24.org>
---
environment.c | 2 +-
pager.c | 11 +++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/environment.c b/environment.c
index ca72464..cfb56fd 100644
--- a/environment.c
+++ b/environment.c
@@ -41,7 +41,7 @@ size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
size_t delta_base_cache_limit = 96 * 1024 * 1024;
unsigned long big_file_threshold = 512 * 1024 * 1024;
const char *pager_program;
-int pager_use_color = 1;
+int pager_use_color = -1;
const char *editor_program;
const char *askpass_program;
const char *excludes_file;
diff --git a/pager.c b/pager.c
index 4bc0481..3110df4 100644
--- a/pager.c
+++ b/pager.c
@@ -80,6 +80,17 @@ void setup_pager(void)
if (!pager)
return;
+ if (pager_use_color < 0 && !strcmp(pager, "more")) {
+ const char *more = getenv("MORE");
+
+ /*
+* MORE=R does not work everywhere, so we cannot set it,
+* but we can respect it if set.
+*/
+ if (!more || !strchr(more, 'R'))
+ pager_use_color = 0;
+ }
+
/*
* force computing the width of the terminal before we redirect
* the standard output to the pager.
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
