# [PATCH] pager: config variable pager.color 8 messages from 2006-07-29 to 2006-07-31. Participants: Matthias Lederhofer, Johannes Schindelin, Junio C Hamano, Juergen Ruehle. Thread: https://gitlist.dev/t/5040 ## Matthias Lederhofer, 2006-07-29 22:27 Subject: [PATCH] pager: config variable pager.color Message-ID: URL: https://gitlist.dev/e/E1G6xHb-0008Rw-G2%40moooo.ath.cx ``` enable/disable colored output when the pager is in use Signed-off-by: Matthias Lederhofer --- setup_pager has to be called before git_diff_ui_config because the latter uses pager_use_color initialized by setup_pager. --- Documentation/config.txt | 4 ++++ builtin-log.c | 4 +++- cache.h | 1 + diff.c | 2 +- environment.c | 1 + pager.c | 4 ++++ 6 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 96429b6..5e243a8 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -121,6 +121,10 @@ pager.program:: variable first, then the 'PAGER' environment variable and "less" as fallback. +pager.color:: + A boolean to enable/disable colored output when the pager is in + use (default is true). + diff.color:: When true (or `always`), always use colors in patch. When false (or `never`), never. When set to `auto`, use diff --git a/builtin-log.c b/builtin-log.c index 82c69d1..7fdefec 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -34,7 +34,6 @@ static int cmd_log_walk(struct rev_info struct commit *commit; prepare_revision_walk(rev); - setup_pager(); while ((commit = get_revision(rev)) != NULL) { log_tree_commit(rev, commit); free(commit->buffer); @@ -49,6 +48,7 @@ int cmd_whatchanged(int argc, const char { struct rev_info rev; + setup_pager(); git_config(git_diff_ui_config); init_revisions(&rev, prefix); rev.diff = 1; @@ -64,6 +64,7 @@ int cmd_show(int argc, const char **argv { struct rev_info rev; + setup_pager(); git_config(git_diff_ui_config); init_revisions(&rev, prefix); rev.diff = 1; @@ -81,6 +82,7 @@ int cmd_log(int argc, const char **argv, { struct rev_info rev; + setup_pager(); git_config(git_diff_ui_config); init_revisions(&rev, prefix); rev.always_show_header = 1; diff --git a/cache.h b/cache.h index 8891073..913be6a 100644 --- a/cache.h +++ b/cache.h @@ -392,6 +392,7 @@ extern int receive_keep_pack(int fd[2], /* pager.c */ extern void setup_pager(void); extern int pager_in_use; +extern int pager_use_color; /* base85 */ int decode_85(char *dst, char *line, int linelen); diff --git a/diff.c b/diff.c index 6198a61..16d47bf 100644 --- a/diff.c +++ b/diff.c @@ -175,7 +175,7 @@ int git_diff_ui_config(const char *var, diff_use_color_default = 1; /* bool */ else if (!strcasecmp(value, "auto")) { diff_use_color_default = 0; - if (isatty(1) || pager_in_use) { + if (isatty(1) || (pager_in_use && pager_use_color)) { char *term = getenv("TERM"); if (term && strcmp(term, "dumb")) diff_use_color_default = 1; diff --git a/environment.c b/environment.c index 558801a..1ce3411 100644 --- a/environment.c +++ b/environment.c @@ -23,6 +23,7 @@ int shared_repository = PERM_UMASK; const char *apply_default_whitespace = NULL; int zlib_compression_level = Z_DEFAULT_COMPRESSION; int pager_in_use; +int pager_use_color = 1; static int dyn_git_object_dir, dyn_git_index_file, dyn_git_graft_file; static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir, diff --git a/pager.c b/pager.c index 3f753f6..afd684e 100644 --- a/pager.c +++ b/pager.c @@ -14,6 +14,10 @@ static int git_pager_config(const char * pager = strdup(value); return 0; } + if (!strcmp(var, "pager.color")) { + pager_use_color = git_config_bool(var,value); + return 0; + } return 0; } -- 1.4.2.rc2.g91b7 ``` ## Johannes Schindelin, 2006-07-29 23:15 Subject: Re: [PATCH] pager: config variable pager.color Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0607300112340.29667%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` Hi, On Sun, 30 Jul 2006, Matthias Lederhofer wrote: > diff --git a/builtin-log.c b/builtin-log.c > index 82c69d1..7fdefec 100644 > --- a/builtin-log.c > +++ b/builtin-log.c > @@ -34,7 +34,6 @@ static int cmd_log_walk(struct rev_info > struct commit *commit; > > prepare_revision_walk(rev); > - setup_pager(); > while ((commit = get_revision(rev)) != NULL) { > log_tree_commit(rev, commit); > free(commit->buffer); > @@ -49,6 +48,7 @@ int cmd_whatchanged(int argc, const char > { > struct rev_info rev; > > + setup_pager(); > git_config(git_diff_ui_config); > init_revisions(&rev, prefix); > rev.diff = 1; > @@ -64,6 +64,7 @@ int cmd_show(int argc, const char **argv > { > struct rev_info rev; > > + setup_pager(); > git_config(git_diff_ui_config); > init_revisions(&rev, prefix); > rev.diff = 1; > @@ -81,6 +82,7 @@ int cmd_log(int argc, const char **argv, > { > struct rev_info rev; > > + setup_pager(); > git_config(git_diff_ui_config); > init_revisions(&rev, prefix); > rev.always_show_header = 1; Why? The three users of cmd_log_walk() need to call setup_pager() explicitely, when cmd_log_walk() can do it for them? Oh, and I do not really understand why you would enable color _at all_ if you want to disable it when paging. Do you have many instances when you want a color diff which is short enough not to be paged? Ciao, Dscho ``` ## Matthias Lederhofer, 2006-07-30 00:43 Subject: Re: [PATCH] pager: config variable pager.color Message-ID: URL: https://gitlist.dev/e/E1G6zPH-00062L-Je%40moooo.ath.cx In-Reply-To: ``` Johannes Schindelin wrote: > Why? The three users of cmd_log_walk() need to call setup_pager() > explicitely, when cmd_log_walk() can do it for them? The explanation is below the commit message: "setup_pager has to be called before git_diff_ui_config because the latter uses pager_use_color initialized by setup_pager." > Oh, and I do not really understand why you would enable color _at all_ if > you want to disable it when paging. Do you have many instances when you > want a color diff which is short enough not to be paged? When I use a pager that escapes the escape character or highlights the content itself the output of git diff without the pager should have colors but not with the pager. For example using git diff with a pathspec is quite short most of the time. For git diff I have to enable paging manually and run git diff | $PAGER usually but git log uses the pager automatically and should not use colors with it. ``` ## Johannes Schindelin, 2006-07-30 09:33 Subject: Re: [PATCH] pager: config variable pager.color Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0607301132320.29667%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` Hi, On Sun, 30 Jul 2006, Matthias Lederhofer wrote: > Johannes Schindelin wrote: > > Why? The three users of cmd_log_walk() need to call setup_pager() > > explicitely, when cmd_log_walk() can do it for them? > > The explanation is below the commit message: > "setup_pager has to be called before git_diff_ui_config because the > latter uses pager_use_color initialized by setup_pager." > > > Oh, and I do not really understand why you would enable color _at all_ if > > you want to disable it when paging. Do you have many instances when you > > want a color diff which is short enough not to be paged? > When I use a pager that escapes the escape character or highlights the > content itself the output of git diff without the pager should have > colors but not with the pager. For example using git diff with a > pathspec is quite short most of the time. For git diff I have to > enable paging manually and run git diff | $PAGER usually but git log > uses the pager automatically and should not use colors with it. I understand now. Maybe you want to put such reasoning into the commit message next time? Ciao, Dscho ``` ## Junio C Hamano, 2006-07-31 00:43 Subject: Re: [PATCH] pager: config variable pager.color Message-ID: <7vd5bmbo46.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vd5bmbo46.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Matthias Lederhofer writes: > Johannes Schindelin wrote: >> Why? The three users of cmd_log_walk() need to call setup_pager() >> explicitely, when cmd_log_walk() can do it for them? > > The explanation is below the commit message: > "setup_pager has to be called before git_diff_ui_config because the > latter uses pager_use_color initialized by setup_pager." If that is the reason, perhaps we could restructure the setting and use of of diff_use_color_default like the attached, which would be cleaner. -- >8 -- diff --git a/cache.h b/cache.h index 8891073..913be6a 100644 --- a/cache.h +++ b/cache.h @@ -392,6 +392,7 @@ extern int receive_keep_pack(int fd[2], /* pager.c */ extern void setup_pager(void); extern int pager_in_use; +extern int pager_use_color; /* base85 */ int decode_85(char *dst, char *line, int linelen); diff --git a/diff.c b/diff.c index 6198a61..043ecb1 100644 --- a/diff.c +++ b/diff.c @@ -173,14 +173,8 @@ int git_diff_ui_config(const char *var, if (!strcmp(var, "diff.color")) { if (!value) diff_use_color_default = 1; /* bool */ - else if (!strcasecmp(value, "auto")) { - diff_use_color_default = 0; - if (isatty(1) || pager_in_use) { - char *term = getenv("TERM"); - if (term && strcmp(term, "dumb")) - diff_use_color_default = 1; - } - } + else if (!strcasecmp(value, "auto")) + diff_use_color_default = -1; /* decide in setup */ else if (!strcasecmp(value, "never")) diff_use_color_default = 0; else if (!strcasecmp(value, "always")) @@ -1509,6 +1503,14 @@ void diff_setup(struct diff_options *opt options->change = diff_change; options->add_remove = diff_addremove; + if (diff_use_color_default < 0) { + diff_use_color_default = 0; + if (isatty(1) || (pager_in_use && pager_use_color)) { + char *term = getenv("TERM"); + if (term && strcmp(term, "dumb")) + diff_use_color_default = 1; + } + } options->color_diff = diff_use_color_default; options->detect_rename = diff_detect_rename_default; } diff --git a/environment.c b/environment.c index 558801a..1ce3411 100644 --- a/environment.c +++ b/environment.c @@ -23,6 +23,7 @@ int shared_repository = PERM_UMASK; const char *apply_default_whitespace = NULL; int zlib_compression_level = Z_DEFAULT_COMPRESSION; int pager_in_use; +int pager_use_color = 1; static int dyn_git_object_dir, dyn_git_index_file, dyn_git_graft_file; static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir, diff --git a/pager.c b/pager.c index 3f753f6..e86ea9e 100644 --- a/pager.c +++ b/pager.c @@ -14,6 +14,10 @@ static int git_pager_config(const char * pager = strdup(value); return 0; } + if (!strcmp(var, "pager.color")) { + pager_use_color = git_config_bool(var, value); + return 0; + } return 0; } ``` ## Juergen Ruehle, 2006-07-31 08:55 Subject: Re: [PATCH] pager: config variable pager.color Message-ID: <17613.50574.56000.861072@lapjr.intranet.kiel.bmiag.de> URL: https://gitlist.dev/e/17613.50574.56000.861072%40lapjr.intranet.kiel.bmiag.de In-Reply-To: <7vd5bmbo46.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano writes: > Matthias Lederhofer writes: > > > Johannes Schindelin wrote: > >> Why? The three users of cmd_log_walk() need to call setup_pager() > >> explicitely, when cmd_log_walk() can do it for them? > > > > The explanation is below the commit message: > > "setup_pager has to be called before git_diff_ui_config because the > > latter uses pager_use_color initialized by setup_pager." > > If that is the reason, perhaps we could restructure the setting > and use of of diff_use_color_default like the attached, which > would be cleaner. AFAICS Matthias' patch has the added benefit of moving setup_pager to before large files (i.e. packs) are mapped. This helps non-COW-fork (i.e. cygwin) tremendously. Actually with Linus' setup refactoring this could probably be easily moved to the wrapper, but I'm too stupid so I just deleted setup_pager from my version:-) jr ``` ## Junio C Hamano, 2006-07-31 09:53 Subject: Re: [PATCH] pager: config variable pager.color Message-ID: <7vzmeq6qxh.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vzmeq6qxh.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <17613.50574.56000.861072@lapjr.intranet.kiel.bmiag.de> ``` Juergen Ruehle writes: > AFAICS Matthias' patch has the added benefit of moving setup_pager to > before large files (i.e. packs) are mapped. This helps non-COW-fork > (i.e. cygwin) tremendously. Actually with Linus' setup refactoring > this could probably be easily moved to the wrapper,... Hmph. Never thought about that. Something like this? -- >8 -- Builtins: control the use of pager from the command table. This moves the built-in "always-use-pager" logic for log family to the command dispatch table of git wrapper. This makes it easier to change the default use of pager, and has an added benefit that we fork and exec the pager early before packs are mmapped. Pointed out by Juergen Ruehle . Signed-off-by: Junio C Hamano --- diff --git a/builtin-log.c b/builtin-log.c index 82c69d1..bba1496 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -34,7 +34,6 @@ static int cmd_log_walk(struct rev_info struct commit *commit; prepare_revision_walk(rev); - setup_pager(); while ((commit = get_revision(rev)) != NULL) { log_tree_commit(rev, commit); free(commit->buffer); diff --git a/git.c b/git.c index 7321d6c..d031eb9 100644 --- a/git.c +++ b/git.c @@ -211,6 +211,7 @@ static int handle_alias(int *argcp, cons const char git_version_string[] = GIT_VERSION; #define NEEDS_PREFIX 1 +#define USE_PAGER 2 static void handle_internal_command(int argc, const char **argv, char **envp) { @@ -218,13 +219,13 @@ static void handle_internal_command(int static struct cmd_struct { const char *cmd; int (*fn)(int, const char **, const char *); - int prefix; + int option; } commands[] = { { "version", cmd_version }, { "help", cmd_help }, - { "log", cmd_log, NEEDS_PREFIX }, - { "whatchanged", cmd_whatchanged, NEEDS_PREFIX }, - { "show", cmd_show, NEEDS_PREFIX }, + { "log", cmd_log, NEEDS_PREFIX | USE_PAGER }, + { "whatchanged", cmd_whatchanged, NEEDS_PREFIX | USE_PAGER }, + { "show", cmd_show, NEEDS_PREFIX | USE_PAGER }, { "push", cmd_push }, { "format-patch", cmd_format_patch, NEEDS_PREFIX }, { "count-objects", cmd_count_objects }, @@ -275,8 +276,10 @@ static void handle_internal_command(int continue; prefix = NULL; - if (p->prefix) + if (p->option & NEEDS_PREFIX) prefix = setup_git_directory(); + if (p->option & USE_PAGER) + setup_pager(); if (getenv("GIT_TRACE")) { int i; fprintf(stderr, "trace: built-in: git"); ``` ## Matthias Lederhofer, 2006-07-31 12:02 Subject: Re: [PATCH] pager: config variable pager.color Message-ID: URL: https://gitlist.dev/e/E1G7WTk-0006cb-94%40moooo.ath.cx In-Reply-To: <7vzmeq6qxh.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Hmph. Never thought about that. Something like this? Funny, I wrote this patch yesterday too :) Add signed-off-by if you like. ```