{"thread":{"id":"18978","subject":"[PATCH] grep: don't support \"grep.color\"-like config options","startedAt":"2009-04-20T22:58:15Z","lastAt":"2009-04-21T15:45:29Z","messageCount":6,"participants":["Markus Heidelberg","Johannes Schindelin","Junio C Hamano","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"111795","messageId":"1240268295-10296-1-git-send-email-markus.heidelberg@web.de","threadId":"18978","inReplyTo":null,"subject":"[PATCH] grep: don't support \"grep.color\"-like config options","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-20T22:58:15Z","receivedAt":"2009-04-20T22:58:15Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"color.grep and color.grep.* is the official and documented way to\nhighlight grep matches. Comparable options like diff.color.* and\nstatus.color.* exist for backward compatibility reasons only and are not\ndocumented any more.\n\nSigned-off-by: Markus Heidelberg <markus.heidelberg@web.de>\n---\n builtin-grep.c |    9 +++------\n 1 files changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 89489dd..f88a912 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -26,16 +26,13 @@ static int grep_config(const char *var, const char *value, void *cb)\n {\n \tstruct grep_opt *opt = cb;\n \n-\tif (!strcmp(var, \"grep.color\") || !strcmp(var, \"color.grep\")) {\n+\tif (!strcmp(var, \"color.grep\")) {\n \t\topt->color = git_config_colorbool(var, value, -1);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(var, \"grep.color.external\") ||\n-\t    !strcmp(var, \"color.grep.external\")) {\n+\tif (!strcmp(var, \"color.grep.external\"))\n \t\treturn git_config_string(&(opt->color_external), var, value);\n-\t}\n-\tif (!strcmp(var, \"grep.color.match\") ||\n-\t    !strcmp(var, \"color.grep.match\")) {\n+\tif (!strcmp(var, \"color.grep.match\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n \t\tcolor_parse(value, var, opt->color_match);\n-- \n1.6.3.rc1.44.g269729\n"},{"id":"111796","messageId":"alpine.DEB.1.00.0904210145260.10279@pacific.mpi-cbg.de","threadId":"18978","inReplyTo":"1240268295-10296-1-git-send-email-markus.heidelberg@web.de","subject":"Re: [PATCH] grep: don't support \"grep.color\"-like config options","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-20T23:45:41Z","receivedAt":"2009-04-20T23:45:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 21 Apr 2009, Markus Heidelberg wrote:\n\n> color.grep and color.grep.* is the official and documented way to \n> highlight grep matches. Comparable options like diff.color.* and \n> status.color.* exist for backward compatibility reasons only and are not \n> documented any more.\n\nBut is it really so bad that we have to possibly break existing setups?\n\nThe cost of keeping them is really small IMHO.\n\nCiao,\nDscho\n"},{"id":"111797","messageId":"200904210211.52818.markus.heidelberg@web.de","threadId":"18978","inReplyTo":"alpine.DEB.1.00.0904210145260.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] grep: don't support \"grep.color\"-like config options","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-21T00:11:52Z","receivedAt":"2009-04-21T00:11:52Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Johannes Schindelin, 21.04.2009:\n> Hi,\n> \n> On Tue, 21 Apr 2009, Markus Heidelberg wrote:\n> \n> > color.grep and color.grep.* is the official and documented way to \n> > highlight grep matches. Comparable options like diff.color.* and \n> > status.color.* exist for backward compatibility reasons only and are not \n> > documented any more.\n> \n> But is it really so bad that we have to possibly break existing setups?\n\nGiven that they have never been documented since introduction of \"grep\n--color\" in March, wouldn't it be pure luck from a users' POV, that they\neven worked, after the user accidentally typed grep.color instead of\ncolor.grep with git-config?\nShould we then also introduce branch.color afterwards?\n\nAlso, colorized grep isn't yet released, people currently using\ngrep.color (and I guess they are very few, if any) normally are\ncompiling git on their own and shouldn't have problems with it\ndisappearing.\n\n> The cost of keeping them is really small IMHO.\n\nMaybe, but it's always nice to keep things simpler and cleaner.\n\nMarkus\n"},{"id":"111842","messageId":"7vy6tua6uh.fsf@gitster.siamese.dyndns.org","threadId":"18978","inReplyTo":"alpine.DEB.1.00.0904210145260.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] grep: don't support \"grep.color\"-like config options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-21T05:55:34Z","receivedAt":"2009-04-21T05:55:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Tue, 21 Apr 2009, Markus Heidelberg wrote:\n>\n>> color.grep and color.grep.* is the official and documented way to \n>> highlight grep matches. Comparable options like diff.color.* and \n>> status.color.* exist for backward compatibility reasons only and are not \n>> documented any more.\n>\n> But is it really so bad that we have to possibly break existing setups?\n>\n> The cost of keeping them is really small IMHO.\n\nWhile I do not think these lines themselves will cost us a lot of\nmaintenance effort, people tend to mimick existing code, and these\npatterns can proliferate if unchecked, and *that* would add to\nmaintenance.  As the feature has aleady been in master for 6 weeks or so,\nMarkus's patch is now-or-never, and I'd rather say we take it before the\n1.6.3 final.\n\nAn alternative would be to:\n\n (1) Keep them undocumented;\n\n (2) add code comments about their deprecated status to discourage people\n     from copying the style; and\n\n (3) possibly deprecate it in some future.\n\nbut I do not know if the last step is worth it.\n\nIn any case, I think perhaps we should squash this in.\n\n diff.c      |    5 +++++\n wt-status.c |    5 +++++\n 2 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 3ac7168..f2432de 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -79,6 +79,11 @@ static int git_config_rename(const char *var, const char *value)\n  */\n int git_diff_ui_config(const char *var, const char *value, void *cb)\n {\n+\t/*\n+\t * Note: cmdname.color style of configuration variables are\n+\t * deprecated; do not copy this pattern but accept only\n+\t * color.cmdname in new code.\n+\t */\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\")) {\n \t\tdiff_use_color_default = git_config_colorbool(var, value, -1);\n \t\treturn 0;\ndiff --git a/wt-status.c b/wt-status.c\nindex 929b00f..9726e0b 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -381,6 +381,11 @@ int git_status_config(const char *k, const char *v, void *cb)\n \t\t\twt_status_submodule_summary = -1;\n \t\treturn 0;\n \t}\n+\t/*\n+\t * Note: cmdname.color style of configuration variables are\n+\t * deprecated; do not copy this pattern but accept only\n+\t * color.cmdname in new code.\n+\t */\n \tif (!strcmp(k, \"status.color\") || !strcmp(k, \"color.status\")) {\n \t\twt_status_use_color = git_config_colorbool(k, v, -1);\n \t\treturn 0;\n"},{"id":"111848","messageId":"200904210957.55640.markus.heidelberg@web.de","threadId":"18978","inReplyTo":"7vy6tua6uh.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] grep: don't support \"grep.color\"-like config options","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-21T07:57:54Z","receivedAt":"2009-04-21T07:57:54Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Junio C Hamano, 21.04.2009:\n>  (2) add code comments about their deprecated status to discourage people\n>      from copying the style; and\n\nGood idea.\n\n> In any case, I think perhaps we should squash this in.\n> \n>  diff.c      |    5 +++++\n>  wt-status.c |    5 +++++\n>  2 files changed, 10 insertions(+), 0 deletions(-)\n\nThere is still pager.color in config.c\n\nMarkus\n"},{"id":"111882","messageId":"49EDEA19.2000900@lsrfire.ath.cx","threadId":"18978","inReplyTo":"7vy6tua6uh.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] grep: don't support \"grep.color\"-like config options","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-04-21T15:45:29Z","receivedAt":"2009-04-21T15:45:29Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Junio C Hamano schrieb:\n> While I do not think these lines themselves will cost us a lot of\n> maintenance effort, people tend to mimick existing code, and these\n> patterns can proliferate if unchecked, and *that* would add to\n> maintenance.\n\nI have to admit that I blindly copied these option names from diff to grep..\n\n> As the feature has aleady been in master for 6 weeks or so,\n> Markus's patch is now-or-never, and I'd rather say we take it before the\n> 1.6.3 final.\n\nAgreed.\n\nRené\n"}]}