{"thread":{"id":"48951","subject":"[PATCH] diff: --color-moved: rename \"dimmed_zebra\" to \"dimmed-zebra\"","startedAt":"2018-07-24T21:59:10Z","lastAt":"2018-07-25T08:21:33Z","messageCount":4,"participants":["Eric Sunshine","Jonathan Nieder","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"353537","messageId":"20180724215845.7137-1-sunshine@sunshineco.com","threadId":"48951","inReplyTo":null,"subject":"[PATCH] diff: --color-moved: rename \"dimmed_zebra\" to \"dimmed-zebra\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-24T21:58:45Z","receivedAt":"2018-07-24T21:59:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The --color-moved \"dimmed_zebra\" mode (with an underscore) is an\nanachronism. Most options and modes are hyphenated. It is more difficult\nto type and somewhat more difficult to read than those which are\nhyphenated. Therefore, rename it to \"dimmed-zebra\", and nominally\ndeprecate \"dimmed_zebra\".\n\nSigned-off-by: Eric Sunshine \n---\n Documentation/diff-options.txt | 3 ++-\n diff.c | 4 +++-\n t/t4015-diff-whitespace.sh | 4 ++--\n 3 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 41064909ee..9195bcd398 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -282,10 +282,11 @@ zebra::\n \tpainted using either the 'color.diff.{old,new}Moved' color or\n \t'color.diff.{old,new}MovedAlternative'. The change between\n \tthe two colors indicates that a new block was detected.\n-dimmed_zebra::\n+dimmed-zebra::\n \tSimilar to 'zebra', but additional dimming of uninteresting parts\n \tof moved code is performed. The bordering lines of two adjacent\n \tblocks are considered interesting, the rest is uninteresting.\n+\t`dimmed_zebra` is a deprecated synonym.\n --\n \n --word-diff[=]::\ndiff --git a/diff.c b/diff.c\nindex dc53a19bab..4ffabb4965 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -268,10 +268,12 @@ static int parse_color_moved(const char *arg)\n \t\treturn COLOR_MOVED_ZEBRA;\n \telse if (!strcmp(arg, \"default\"))\n \t\treturn COLOR_MOVED_DEFAULT;\n+\telse if (!strcmp(arg, \"dimmed-zebra\"))\n+\t\treturn COLOR_MOVED_ZEBRA_DIM;\n \telse if (!strcmp(arg, \"dimmed_zebra\"))\n \t\treturn COLOR_MOVED_ZEBRA_DIM;\n \telse\n-\t\treturn error(_(\"color moved setting must be one of 'no', 'default', 'zebra', 'dimmed_zebra', 'plain'\"));\n+\t\treturn error(_(\"color moved setting must be one of 'no', 'default', 'zebra', 'dimmed-zebra', 'plain'\"));\n }\n \n int git_diff_ui_config(const char *var, const char *value, void *cb)\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 17df491a3a..8cdfa225ef 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -1223,7 +1223,7 @@ test_expect_success 'plain moved code, inside file' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '\n+test_expect_success 'detect permutations inside moved code -- dimmed-zebra' '\n \tgit reset --hard &&\n \tcat <<-\\EOF >lines.txt &&\n \t\tlong line 1\n@@ -1271,7 +1271,7 @@ test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '\n \ttest_config color.diff.newMovedDimmed \"normal cyan\" &&\n \ttest_config color.diff.oldMovedAlternativeDimmed \"normal blue\" &&\n \ttest_config color.diff.newMovedAlternativeDimmed \"normal yellow\" &&\n-\tgit diff HEAD --no-renames --color-moved=dimmed_zebra --color |\n+\tgit diff HEAD --no-renames --color-moved=dimmed-zebra --color |\n \t\tgrep -v \"index\" |\n \t\ttest_decode_color >actual &&\n \tcat <<-\\EOF >expected &&\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353540","messageId":"20180724220928.GE136514@aiede.svl.corp.google.com","threadId":"48951","inReplyTo":"20180724215845.7137-1-sunshine@sunshineco.com","subject":"Re: [PATCH] diff: --color-moved: rename \"dimmed_zebra\" to \"dimmed-zebra\"","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-07-24T22:09:28Z","receivedAt":"2018-07-24T22:09:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Eric Sunshine wrote:\n\n> Subject: diff: --color-moved: rename \"dimmed_zebra\" to \"dimmed-zebra\"\n\nIt would be clearer to say something like\n\n\tdiff --color-moved: add \"dimmed-zebra\" synonym for \"dimmed_zebra\"\n\n> The --color-moved \"dimmed_zebra\" mode (with an underscore) is an\n> anachronism. Most options and modes are hyphenated. It is more difficult\n> to type and somewhat more difficult to read than those which are\n> hyphenated. Therefore, rename it to \"dimmed-zebra\", and nominally\n> deprecate \"dimmed_zebra\".\n\nHm. Looks like dimmed_zebra was introduced in v2.15.0-rc0~16^2~2\n(2017-06-30), so it has been around for a while (about a year). But I\nwould like to be able to simplify by getting rid of it.\n\nhttps://codesearch.debian.net/search?q=dimmed_zebra doesn't find any\ninstances of it being used outside Git itself.\n\nhttps://twitter.com/kornelski/status/982247477020508162 (found with a\nweb search) shows that some people may have it in configuration.\n\nIs there anything we can do to make it possible to remove eventually?\nFor example, should we (eventually, after dimmed-zebra has existed\nfor some time) start warning when people use dimmed_zebra to encourage\nthem to use dimmed-zebra instead?\n\n> Signed-off-by: Eric Sunshine \n> ---\n> Documentation/diff-options.txt | 3 ++-\n> diff.c | 4 +++-\n> t/t4015-diff-whitespace.sh | 4 ++--\n> 3 files changed, 7 insertions(+), 4 deletions(-)\n> \n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 41064909ee..9195bcd398 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -282,10 +282,11 @@ zebra::\n> \tpainted using either the 'color.diff.{old,new}Moved' color or\n> \t'color.diff.{old,new}MovedAlternative'. The change between\n> \tthe two colors indicates that a new block was detected.\n> -dimmed_zebra::\n> +dimmed-zebra::\n> \tSimilar to 'zebra', but additional dimming of uninteresting parts\n> \tof moved code is performed. The bordering lines of two adjacent\n> \tblocks are considered interesting, the rest is uninteresting.\n> +\t`dimmed_zebra` is a deprecated synonym.\n\nThanks for including that note. It means that when people see\ndimmed_zebra in scripts or configuration they won't have to be\nmystified about why it works.\n\nI don't have any good ideas about deprecating more aggressively, and\nthe patch looks good, so\n\nReviewed-by: Jonathan Nieder \n\nThanks.\n"},{"id":"353544","messageId":"CAGZ79kbL2mHK9TzH6guRjJkzmaVSbMiPr5M47RPMtYzp782iFQ@mail.gmail.com","threadId":"48951","inReplyTo":"20180724220928.GE136514@aiede.svl.corp.google.com","subject":"Re: [PATCH] diff: --color-moved: rename \"dimmed_zebra\" to \"dimmed-zebra\"","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-24T22:42:48Z","receivedAt":"2018-07-24T22:43:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jul 24, 2018 at 3:09 PM Jonathan Nieder wrote:\n> Hm. Looks like dimmed_zebra was introduced in v2.15.0-rc0~16^2~2\n> (2017-06-30), so it has been around for a while (about a year). But I\n> would like to be able to simplify by getting rid of it.\n>\n> https://codesearch.debian.net/search?q=dimmed_zebra doesn't find any\n> instances of it being used outside Git itself.\n>\n> https://twitter.com/kornelski/status/982247477020508162 (found with a\n> web search) shows that some people may have it in configuration.\n\nThanks for the searches.\n\n>\n> I don't have any good ideas about deprecating more aggressively, and\n> the patch looks good, so\n>\n> Reviewed-by: Jonathan Nieder \n\nSame here. I wonder if we actually ever want to deprecate it further than\nthis patch, as the additional code to support it is only 2 lines.\nSo the effort to deprecate it (more than this patch) is more than\nkeeping the misnamed version around forever.\n\nThanks!\nStefan\n"},{"id":"353549","messageId":"CAPig+cQ2BWc6c8pv9+Cb+=HLdo0KAtAY2N08ozaRVVYWSqG3GQ@mail.gmail.com","threadId":"48951","inReplyTo":"20180724220928.GE136514@aiede.svl.corp.google.com","subject":"Re: [PATCH] diff: --color-moved: rename \"dimmed_zebra\" to \"dimmed-zebra\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-25T08:21:19Z","receivedAt":"2018-07-25T08:21:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 24, 2018 at 6:09 PM Jonathan Nieder wrote:\n> Eric Sunshine wrote:\n> > Subject: diff: --color-moved: rename \"dimmed_zebra\" to \"dimmed-zebra\"\n>\n> It would be clearer to say something like\n>\n> diff --color-moved: add \"dimmed-zebra\" synonym for \"dimmed_zebra\"\n\nThat could work, although doesn't really convey that \"dimmed_zebra\" is\nundesirable. Perhaps:\n\n diff --color-moved: add \"dimmed-zebra\"; deprecate \"dimmed_zebra\"\n\nPerhaps, not worth a re-roll, though(?).\n\n> > The --color-moved \"dimmed_zebra\" mode (with an underscore) is an\n> > anachronism. Most options and modes are hyphenated. It is more difficult\n> > to type and somewhat more difficult to read than those which are\n> > hyphenated. Therefore, rename it to \"dimmed-zebra\", and nominally\n> > deprecate \"dimmed_zebra\".\n>\n> Hm. Looks like dimmed_zebra was introduced in v2.15.0-rc0~16^2~2\n> (2017-06-30), so it has been around for a while (about a year). But I\n> would like to be able to simplify by getting rid of it.\n>\n> Is there anything we can do to make it possible to remove eventually?\n> For example, should we (eventually, after dimmed-zebra has existed\n> for some time) start warning when people use dimmed_zebra to encourage\n> them to use dimmed-zebra instead?\n\nWanting to simplify by eventually getting rid of the old name is\nunderstandable, but my hope is that we don't have to do so. Given that\nthis nominal deprecation involves one short sentence in the\ndocumentation and one strcmp() for backward compatibility, it seems\nunlikely to become a maintenance burden. In contrast to \"git branch\n-l\", short for --create-reflog, but which people intuitively expect to\nmean --list, \"freeing up\" this name for some other purpose is\nunnecessary, so a concrete deprecation may be overkill. As well, the\nextra burden on Junio to hang onto deprecation and eventual removal\npatches far into the future seems unwarranted.\n\n> > +dimmed-zebra::\n> > Similar to 'zebra', but additional dimming of uninteresting parts\n> > of moved code is performed. The bordering lines of two adjacent\n> > blocks are considered interesting, the rest is uninteresting.\n> > + `dimmed_zebra` is a deprecated synonym.\n>\n> Thanks for including that note. It means that when people see\n> dimmed_zebra in scripts or configuration they won't have to be\n> mystified about why it works.\n>\n> I don't have any good ideas about deprecating more aggressively, and\n> the patch looks good, so\n>\n> Reviewed-by: Jonathan Nieder \n\nThanks for the review.\n"}]}