{"thread":{"id":"5040","subject":"[PATCH] pager: config variable pager.color","startedAt":"2006-07-29T22:27:43Z","lastAt":"2006-07-31T12:02:36Z","messageCount":8,"participants":["Matthias Lederhofer","Johannes Schindelin","Junio C Hamano","Juergen Ruehle"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"24353","messageId":"E1G6xHb-0008Rw-G2@moooo.ath.cx","threadId":"5040","inReplyTo":null,"subject":"[PATCH] pager: config variable pager.color","fromName":"Matthias Lederhofer","fromEmail":"matled@gmx.net","sentAt":"2006-07-29T22:27:43Z","receivedAt":"2006-07-29T22:27:43Z","isPatch":true,"sender":{"key":"matled@gmx.net","avatar":null},"body":"enable/disable colored output when the pager is in use\n\nSigned-off-by: Matthias Lederhofer \n---\nsetup_pager has to be called before git_diff_ui_config because the\nlatter uses pager_use_color initialized by setup_pager.\n---\n Documentation/config.txt | 4 ++++\n builtin-log.c | 4 +++-\n cache.h | 1 +\n diff.c | 2 +-\n environment.c | 1 +\n pager.c | 4 ++++\n 6 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 96429b6..5e243a8 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -121,6 +121,10 @@ pager.program::\n \tvariable first, then the 'PAGER' environment variable and\n \t\"less\" as fallback.\n \n+pager.color::\n+\tA boolean to enable/disable colored output when the pager is in\n+\tuse (default is true).\n+\n diff.color::\n \tWhen true (or `always`), always use colors in patch.\n \tWhen false (or `never`), never. When set to `auto`, use\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 82c69d1..7fdefec 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -34,7 +34,6 @@ static int cmd_log_walk(struct rev_info \n \tstruct commit *commit;\n \n \tprepare_revision_walk(rev);\n-\tsetup_pager();\n \twhile ((commit = get_revision(rev)) != NULL) {\n \t\tlog_tree_commit(rev, commit);\n \t\tfree(commit->buffer);\n@@ -49,6 +48,7 @@ int cmd_whatchanged(int argc, const char\n {\n \tstruct rev_info rev;\n \n+\tsetup_pager();\n \tgit_config(git_diff_ui_config);\n \tinit_revisions(&rev, prefix);\n \trev.diff = 1;\n@@ -64,6 +64,7 @@ int cmd_show(int argc, const char **argv\n {\n \tstruct rev_info rev;\n \n+\tsetup_pager();\n \tgit_config(git_diff_ui_config);\n \tinit_revisions(&rev, prefix);\n \trev.diff = 1;\n@@ -81,6 +82,7 @@ int cmd_log(int argc, const char **argv,\n {\n \tstruct rev_info rev;\n \n+\tsetup_pager();\n \tgit_config(git_diff_ui_config);\n \tinit_revisions(&rev, prefix);\n \trev.always_show_header = 1;\ndiff --git a/cache.h b/cache.h\nindex 8891073..913be6a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -392,6 +392,7 @@ extern int receive_keep_pack(int fd[2], \n /* pager.c */\n extern void setup_pager(void);\n extern int pager_in_use;\n+extern int pager_use_color;\n \n /* base85 */\n int decode_85(char *dst, char *line, int linelen);\ndiff --git a/diff.c b/diff.c\nindex 6198a61..16d47bf 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -175,7 +175,7 @@ int git_diff_ui_config(const char *var, \n \t\t\tdiff_use_color_default = 1; /* bool */\n \t\telse if (!strcasecmp(value, \"auto\")) {\n \t\t\tdiff_use_color_default = 0;\n-\t\t\tif (isatty(1) || pager_in_use) {\n+\t\t\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n \t\t\t\tchar *term = getenv(\"TERM\");\n \t\t\t\tif (term && strcmp(term, \"dumb\"))\n \t\t\t\t\tdiff_use_color_default = 1;\ndiff --git a/environment.c b/environment.c\nindex 558801a..1ce3411 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -23,6 +23,7 @@ int shared_repository = PERM_UMASK;\n const char *apply_default_whitespace = NULL;\n int zlib_compression_level = Z_DEFAULT_COMPRESSION;\n int pager_in_use;\n+int pager_use_color = 1;\n \n static int dyn_git_object_dir, dyn_git_index_file, dyn_git_graft_file;\n static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir,\ndiff --git a/pager.c b/pager.c\nindex 3f753f6..afd684e 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -14,6 +14,10 @@ static int git_pager_config(const char *\n \t\t\tpager = strdup(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"pager.color\")) {\n+\t\tpager_use_color = git_config_bool(var,value);\n+\t\treturn 0;\n+\t}\n \treturn 0;\n }\n \n-- \n1.4.2.rc2.g91b7\n"},{"id":"24355","messageId":"Pine.LNX.4.63.0607300112340.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5040","inReplyTo":"E1G6xHb-0008Rw-G2@moooo.ath.cx","subject":"Re: [PATCH] pager: config variable pager.color","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-29T23:15:31Z","receivedAt":"2006-07-29T23:15:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 30 Jul 2006, Matthias Lederhofer wrote:\n\n> diff --git a/builtin-log.c b/builtin-log.c\n> index 82c69d1..7fdefec 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -34,7 +34,6 @@ static int cmd_log_walk(struct rev_info \n> \tstruct commit *commit;\n> \n> \tprepare_revision_walk(rev);\n> -\tsetup_pager();\n> \twhile ((commit = get_revision(rev)) != NULL) {\n> \t\tlog_tree_commit(rev, commit);\n> \t\tfree(commit->buffer);\n> @@ -49,6 +48,7 @@ int cmd_whatchanged(int argc, const char\n> {\n> \tstruct rev_info rev;\n> \n> +\tsetup_pager();\n> \tgit_config(git_diff_ui_config);\n> \tinit_revisions(&rev, prefix);\n> \trev.diff = 1;\n> @@ -64,6 +64,7 @@ int cmd_show(int argc, const char **argv\n> {\n> \tstruct rev_info rev;\n> \n> +\tsetup_pager();\n> \tgit_config(git_diff_ui_config);\n> \tinit_revisions(&rev, prefix);\n> \trev.diff = 1;\n> @@ -81,6 +82,7 @@ int cmd_log(int argc, const char **argv,\n> {\n> \tstruct rev_info rev;\n> \n> +\tsetup_pager();\n> \tgit_config(git_diff_ui_config);\n> \tinit_revisions(&rev, prefix);\n> \trev.always_show_header = 1;\n\nWhy? The three users of cmd_log_walk() need to call setup_pager() \nexplicitely, when cmd_log_walk() can do it for them?\n\nOh, and I do not really understand why you would enable color _at all_ if \nyou want to disable it when paging. Do you have many instances when you \nwant a color diff which is short enough not to be paged?\n\nCiao,\nDscho\n"},{"id":"24357","messageId":"E1G6zPH-00062L-Je@moooo.ath.cx","threadId":"5040","inReplyTo":"Pine.LNX.4.63.0607300112340.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] pager: config variable pager.color","fromName":"Matthias Lederhofer","fromEmail":"matled@gmx.net","sentAt":"2006-07-30T00:43:47Z","receivedAt":"2006-07-30T00:43:47Z","isPatch":true,"sender":{"key":"matled@gmx.net","avatar":null},"body":"Johannes Schindelin wrote:\n> Why? The three users of cmd_log_walk() need to call setup_pager() \n> explicitely, when cmd_log_walk() can do it for them?\n\nThe explanation is below the commit message:\n\"setup_pager has to be called before git_diff_ui_config because the\nlatter uses pager_use_color initialized by setup_pager.\"\n\n> Oh, and I do not really understand why you would enable color _at all_ if \n> you want to disable it when paging. Do you have many instances when you \n> want a color diff which is short enough not to be paged?\nWhen I use a pager that escapes the escape character or highlights the\ncontent itself the output of git diff without the pager should have\ncolors but not with the pager. For example using git diff with a\npathspec is quite short most of the time. For git diff I have to\nenable paging manually and run git diff | $PAGER usually but git log\nuses the pager automatically and should not use colors with it.\n"},{"id":"24363","messageId":"Pine.LNX.4.63.0607301132320.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5040","inReplyTo":"E1G6zPH-00062L-Je@moooo.ath.cx","subject":"Re: [PATCH] pager: config variable pager.color","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-30T09:33:30Z","receivedAt":"2006-07-30T09:33:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 30 Jul 2006, Matthias Lederhofer wrote:\n\n> Johannes Schindelin wrote:\n> > Why? The three users of cmd_log_walk() need to call setup_pager() \n> > explicitely, when cmd_log_walk() can do it for them?\n> \n> The explanation is below the commit message:\n> \"setup_pager has to be called before git_diff_ui_config because the\n> latter uses pager_use_color initialized by setup_pager.\"\n> \n> > Oh, and I do not really understand why you would enable color _at all_ if \n> > you want to disable it when paging. Do you have many instances when you \n> > want a color diff which is short enough not to be paged?\n\n> When I use a pager that escapes the escape character or highlights the\n> content itself the output of git diff without the pager should have\n> colors but not with the pager. For example using git diff with a\n> pathspec is quite short most of the time. For git diff I have to\n> enable paging manually and run git diff | $PAGER usually but git log\n> uses the pager automatically and should not use colors with it.\n\nI understand now. Maybe you want to put such reasoning into the commit \nmessage next time?\n\nCiao,\nDscho\n"},{"id":"24396","messageId":"7vd5bmbo46.fsf@assigned-by-dhcp.cox.net","threadId":"5040","inReplyTo":"E1G6zPH-00062L-Je@moooo.ath.cx","subject":"Re: [PATCH] pager: config variable pager.color","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-31T00:43:21Z","receivedAt":"2006-07-31T00:43:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Lederhofer writes:\n\n> Johannes Schindelin wrote:\n>> Why? The three users of cmd_log_walk() need to call setup_pager() \n>> explicitely, when cmd_log_walk() can do it for them?\n>\n> The explanation is below the commit message:\n> \"setup_pager has to be called before git_diff_ui_config because the\n> latter uses pager_use_color initialized by setup_pager.\"\n\nIf that is the reason, perhaps we could restructure the setting\nand use of of diff_use_color_default like the attached, which\nwould be cleaner.\n\n-- >8 --\n\ndiff --git a/cache.h b/cache.h\nindex 8891073..913be6a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -392,6 +392,7 @@ extern int receive_keep_pack(int fd[2], \n /* pager.c */\n extern void setup_pager(void);\n extern int pager_in_use;\n+extern int pager_use_color;\n \n /* base85 */\n int decode_85(char *dst, char *line, int linelen);\ndiff --git a/diff.c b/diff.c\nindex 6198a61..043ecb1 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -173,14 +173,8 @@ int git_diff_ui_config(const char *var, \n \tif (!strcmp(var, \"diff.color\")) {\n \t\tif (!value)\n \t\t\tdiff_use_color_default = 1; /* bool */\n-\t\telse if (!strcasecmp(value, \"auto\")) {\n-\t\t\tdiff_use_color_default = 0;\n-\t\t\tif (isatty(1) || pager_in_use) {\n-\t\t\t\tchar *term = getenv(\"TERM\");\n-\t\t\t\tif (term && strcmp(term, \"dumb\"))\n-\t\t\t\t\tdiff_use_color_default = 1;\n-\t\t\t}\n-\t\t}\n+\t\telse if (!strcasecmp(value, \"auto\"))\n+\t\t\tdiff_use_color_default = -1; /* decide in setup */\n \t\telse if (!strcasecmp(value, \"never\"))\n \t\t\tdiff_use_color_default = 0;\n \t\telse if (!strcasecmp(value, \"always\"))\n@@ -1509,6 +1503,14 @@ void diff_setup(struct diff_options *opt\n \n \toptions->change = diff_change;\n \toptions->add_remove = diff_addremove;\n+\tif (diff_use_color_default < 0) {\n+\t\tdiff_use_color_default = 0;\n+\t\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n+\t\t\tchar *term = getenv(\"TERM\");\n+\t\t\tif (term && strcmp(term, \"dumb\"))\n+\t\t\t\tdiff_use_color_default = 1;\n+\t\t}\n+\t}\n \toptions->color_diff = diff_use_color_default;\n \toptions->detect_rename = diff_detect_rename_default;\n }\ndiff --git a/environment.c b/environment.c\nindex 558801a..1ce3411 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -23,6 +23,7 @@ int shared_repository = PERM_UMASK;\n const char *apply_default_whitespace = NULL;\n int zlib_compression_level = Z_DEFAULT_COMPRESSION;\n int pager_in_use;\n+int pager_use_color = 1;\n \n static int dyn_git_object_dir, dyn_git_index_file, dyn_git_graft_file;\n static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir,\ndiff --git a/pager.c b/pager.c\nindex 3f753f6..e86ea9e 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -14,6 +14,10 @@ static int git_pager_config(const char *\n \t\t\tpager = strdup(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"pager.color\")) {\n+\t\tpager_use_color = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \treturn 0;\n }\n \n"},{"id":"24417","messageId":"17613.50574.56000.861072@lapjr.intranet.kiel.bmiag.de","threadId":"5040","inReplyTo":"7vd5bmbo46.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] pager: config variable pager.color","fromName":"Juergen Ruehle","fromEmail":"j.ruehle@bmiag.de","sentAt":"2006-07-31T08:55:42Z","receivedAt":"2006-07-31T08:55:42Z","isPatch":true,"sender":{"key":"j.ruehle@bmiag.de","avatar":null},"body":"Junio C Hamano writes:\n > Matthias Lederhofer writes:\n > \n > > Johannes Schindelin wrote:\n > >> Why? The three users of cmd_log_walk() need to call setup_pager() \n > >> explicitely, when cmd_log_walk() can do it for them?\n > >\n > > The explanation is below the commit message:\n > > \"setup_pager has to be called before git_diff_ui_config because the\n > > latter uses pager_use_color initialized by setup_pager.\"\n > \n > If that is the reason, perhaps we could restructure the setting\n > and use of of diff_use_color_default like the attached, which\n > would be cleaner.\n\nAFAICS Matthias' patch has the added benefit of moving setup_pager to\nbefore large files (i.e. packs) are mapped. This helps non-COW-fork\n(i.e. cygwin) tremendously. Actually with Linus' setup refactoring\nthis could probably be easily moved to the wrapper, but I'm too stupid\nso I just deleted setup_pager from my version:-)\n\n jr\n"},{"id":"24421","messageId":"7vzmeq6qxh.fsf@assigned-by-dhcp.cox.net","threadId":"5040","inReplyTo":"17613.50574.56000.861072@lapjr.intranet.kiel.bmiag.de","subject":"Re: [PATCH] pager: config variable pager.color","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-31T09:53:46Z","receivedAt":"2006-07-31T09:53:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Juergen Ruehle writes:\n\n> AFAICS Matthias' patch has the added benefit of moving setup_pager to\n> before large files (i.e. packs) are mapped. This helps non-COW-fork\n> (i.e. cygwin) tremendously. Actually with Linus' setup refactoring\n> this could probably be easily moved to the wrapper,...\n\nHmph. Never thought about that. Something like this?\n\n-- >8 --\nBuiltins: control the use of pager from the command table.\n\nThis moves the built-in \"always-use-pager\" logic for log family\nto the command dispatch table of git wrapper. This makes it\neasier to change the default use of pager, and has an added\nbenefit that we fork and exec the pager early before packs are\nmmapped.\n\nPointed out by Juergen Ruehle .\n\nSigned-off-by: Junio C Hamano \n---\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 82c69d1..bba1496 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -34,7 +34,6 @@ static int cmd_log_walk(struct rev_info \n \tstruct commit *commit;\n \n \tprepare_revision_walk(rev);\n-\tsetup_pager();\n \twhile ((commit = get_revision(rev)) != NULL) {\n \t\tlog_tree_commit(rev, commit);\n \t\tfree(commit->buffer);\ndiff --git a/git.c b/git.c\nindex 7321d6c..d031eb9 100644\n--- a/git.c\n+++ b/git.c\n@@ -211,6 +211,7 @@ static int handle_alias(int *argcp, cons\n const char git_version_string[] = GIT_VERSION;\n \n #define NEEDS_PREFIX 1\n+#define USE_PAGER 2\n \n static void handle_internal_command(int argc, const char **argv, char **envp)\n {\n@@ -218,13 +219,13 @@ static void handle_internal_command(int \n \tstatic struct cmd_struct {\n \t\tconst char *cmd;\n \t\tint (*fn)(int, const char **, const char *);\n-\t\tint prefix;\n+\t\tint option;\n \t} commands[] = {\n \t\t{ \"version\", cmd_version },\n \t\t{ \"help\", cmd_help },\n-\t\t{ \"log\", cmd_log, NEEDS_PREFIX },\n-\t\t{ \"whatchanged\", cmd_whatchanged, NEEDS_PREFIX },\n-\t\t{ \"show\", cmd_show, NEEDS_PREFIX },\n+\t\t{ \"log\", cmd_log, NEEDS_PREFIX | USE_PAGER },\n+\t\t{ \"whatchanged\", cmd_whatchanged, NEEDS_PREFIX | USE_PAGER },\n+\t\t{ \"show\", cmd_show, NEEDS_PREFIX | USE_PAGER },\n \t\t{ \"push\", cmd_push },\n \t\t{ \"format-patch\", cmd_format_patch, NEEDS_PREFIX },\n \t\t{ \"count-objects\", cmd_count_objects },\n@@ -275,8 +276,10 @@ static void handle_internal_command(int \n \t\t\tcontinue;\n \n \t\tprefix = NULL;\n-\t\tif (p->prefix)\n+\t\tif (p->option & NEEDS_PREFIX)\n \t\t\tprefix = setup_git_directory();\n+\t\tif (p->option & USE_PAGER)\n+\t\t\tsetup_pager();\n \t\tif (getenv(\"GIT_TRACE\")) {\n \t\t\tint i;\n \t\t\tfprintf(stderr, \"trace: built-in: git\");\n"},{"id":"24432","messageId":"E1G7WTk-0006cb-94@moooo.ath.cx","threadId":"5040","inReplyTo":"7vzmeq6qxh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] pager: config variable pager.color","fromName":"Matthias Lederhofer","fromEmail":"matled@gmx.net","sentAt":"2006-07-31T12:02:36Z","receivedAt":"2006-07-31T12:02:36Z","isPatch":true,"sender":{"key":"matled@gmx.net","avatar":null},"body":"Junio C Hamano wrote:\n> Hmph. Never thought about that. Something like this?\nFunny, I wrote this patch yesterday too :) Add signed-off-by if you\nlike.\n"}]}