{"thread":{"id":"49197","subject":"[PATCH] range-diff: update stale summary of --no-dual-color","startedAt":"2018-08-23T02:40:09Z","lastAt":"2018-08-27T17:57:42Z","messageCount":20,"participants":["Kyle Meyer","Jonathan Nieder","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"356336","messageId":"20180823023955.12980-1-kyle@kyleam.com","threadId":"49197","inReplyTo":null,"subject":"[PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2018-08-23T02:39:55Z","receivedAt":"2018-08-23T02:40:09Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"275267937b (range-diff: make dual-color the default mode, 2018-08-13)\nreplaced --dual-color with --no-dual-color but left the option's\nsummary untouched. Rewrite the summary to describe --no-dual-color\nrather than dual-color.\n\nSigned-off-by: Kyle Meyer \n---\n builtin/range-diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex f52d45d9d6..7dc90a5ec3 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"Percentage by which creation is weighted\")),\n \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n-\t\t\t N_(\"color both diff and diff-between-diffs\")),\n+\t\t\t N_(\"restrict coloring to outer diff markers\")),\n \t\tOPT_END()\n \t};\n \tint i, j, res = 0;\n-- \n2.18.0\n\n"},{"id":"356337","messageId":"20180823024719.GG92374@aiede.svl.corp.google.com","threadId":"49197","inReplyTo":"20180823023955.12980-1-kyle@kyleam.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-23T02:47:19Z","receivedAt":"2018-08-23T02:47:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nKyle Meyer wrote:\n\n> 275267937b (range-diff: make dual-color the default mode, 2018-08-13)\n> replaced --dual-color with --no-dual-color but left the option's\n> summary untouched. Rewrite the summary to describe --no-dual-color\n> rather than dual-color.\n>\n> Signed-off-by: Kyle Meyer \n> ---\n> builtin/range-diff.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nThanks, good catch!\n\n> diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n> index f52d45d9d6..7dc90a5ec3 100644\n> --- a/builtin/range-diff.c\n> +++ b/builtin/range-diff.c\n> @@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n> \t\t\t N_(\"Percentage by which creation is weighted\")),\n> \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n> +\t\t\t N_(\"restrict coloring to outer diff markers\")),\n\nWhat is an outer diff marker?\n\nThanks,\nJonathan\n"},{"id":"356339","messageId":"87wosh7pjj.fsf@kyleam.com","threadId":"49197","inReplyTo":"20180823024719.GG92374@aiede.svl.corp.google.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2018-08-23T03:03:12Z","receivedAt":"2018-08-23T03:03:15Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Jonathan Nieder writes:\n\n[...]\n\n> What is an outer diff marker?\n\nThe diff markers from the diff of patches as opposed to the ones from\nthe original patches. I took the term from git-range-diff.txt:\n\n --no-dual-color::\n When the commit diffs differ, `git range-diff` recreates the\n original diffs' coloring, and adds outer -/+ diff markers [...]\n\n Use `--no-dual-color` to revert to color all lines according to the\n outer diff markers (and completely ignore the inner diff when it\n comes to color).\n\n-- \nKyle\n"},{"id":"356340","messageId":"20180823032238.GI92374@aiede.svl.corp.google.com","threadId":"49197","inReplyTo":"87wosh7pjj.fsf@kyleam.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-23T03:22:38Z","receivedAt":"2018-08-23T03:22:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kyle Meyer wrote:\n> Jonathan Nieder writes:\n>> Kyle Meyer wrote:\n\n>>> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n>>> +\t\t\t N_(\"restrict coloring to outer diff markers\")),\n[...]\n>> What is an outer diff marker?\n>\n> The diff markers from the diff of patches as opposed to the ones from\n> the original patches. I took the term from git-range-diff.txt:\n>\n> --no-dual-color::\n> When the commit diffs differ, `git range-diff` recreates the\n> original diffs' coloring, and adds outer -/+ diff markers [...]\n>\n> Use `--no-dual-color` to revert to color all lines according to the\n> outer diff markers (and completely ignore the inner diff when it\n> comes to color).\n\nAha: I think you're missing a few words (e.g. \"color only according to\nouter diff markers\"). Though based on the output, I'm not sure the\nfocus on diff markers captures the difference. (After all, some lines\nare multiple colors in --no-dual-color mode and have no diff markers.)\n\n\"Restrict coloring to outer -/+ diff markers\" would mean that\neverything will be in plain text, except for the minus or plus sign at\nthe beginning of each line. So you'd see a colorful strip on the left\nand everything else monochrome.\n\nI think what you mean is something like \"color only based on the\ndiff-between-diffs\". Or it might be simpler to do something like\nthe following. What do you think?\n\ndiff --git i/builtin/range-diff.c w/builtin/range-diff.c\nindex f52d45d9d6..88c19f48d3 100644\n--- i/builtin/range-diff.c\n+++ w/builtin/range-diff.c\n@@ -20,12 +20,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n {\n \tint creation_factor = 60;\n \tstruct diff_options diffopt = { NULL };\n-\tint simple_color = -1;\n+\tint dual_color = -1;\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"Percentage by which creation is weighted\")),\n-\t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n-\t\t\t N_(\"color both diff and diff-between-diffs\")),\n+\t\tOPT_BOOL(0, \"dual-color\", &dual_color,\n+\t\t\t N_(\"color both diff and diff-between-diffs (default)\")),\n \t\tOPT_END()\n \t};\n \tint i, j, res = 0;\n@@ -63,8 +63,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\t\t options + ARRAY_SIZE(options) - 1, /* OPT_END */\n \t\t\t builtin_range_diff_usage, 0);\n \n-\tif (simple_color < 1) {\n-\t\tif (!simple_color)\n+\tif (dual_color != 0) {\n+\t\tif (dual_color > 0)\n \t\t\t/* force color when --dual-color was used */\n \t\t\tdiffopt.use_color = 1;\n \t\tdiffopt.flags.dual_color_diffed_diffs = 1;\n"},{"id":"356343","messageId":"87d0u9ybhk.fsf@kyleam.com","threadId":"49197","inReplyTo":"20180823032238.GI92374@aiede.svl.corp.google.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2018-08-23T04:04:39Z","receivedAt":"2018-08-23T04:12:04Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Jonathan Nieder writes:\n\n>>>> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n>>>> +\t\t\t N_(\"restrict coloring to outer diff markers\")),\n\n[...]\n\n> Aha: I think you're missing a few words (e.g. \"color only according to\n> outer diff markers\"). Though based on the output, I'm not sure the\n> focus on diff markers captures the difference. (After all, some lines\n> are multiple colors in --no-dual-color mode and have no diff markers.)\n>\n> \"Restrict coloring to outer -/+ diff markers\" would mean that\n> everything will be in plain text, except for the minus or plus sign at\n> the beginning of each line. So you'd see a colorful strip on the left\n> and everything else monochrome.\n\nEh, you're right, it would read like that. Thanks.\n\n> I think what you mean is something like \"color only based on the\n> diff-between-diffs\".\n\nYeah, I that sounds OK to me. I played around with a few different\nsummary lines and couldn't come up with anything that I thought was\nparticularly good, and then of course I ended up settling on a summary\nline that didn't preserve my intended meaning :/\n\n> Or it might be simpler to do something like\n> the following. What do you think?\n>\n> diff --git i/builtin/range-diff.c w/builtin/range-diff.c\n> index f52d45d9d6..88c19f48d3 100644\n> --- i/builtin/range-diff.c\n> +++ w/builtin/range-diff.c\n> @@ -20,12 +20,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n> {\n> \tint creation_factor = 60;\n> \tstruct diff_options diffopt = { NULL };\n> -\tint simple_color = -1;\n> +\tint dual_color = -1;\n> \tstruct option options[] = {\n> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n> \t\t\t N_(\"Percentage by which creation is weighted\")),\n> -\t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n> +\t\tOPT_BOOL(0, \"dual-color\", &dual_color,\n> +\t\t\t N_(\"color both diff and diff-between-diffs (default)\")),\n\nI don't have a strong preference, though I lean towards making 'git\nrange-diff -h' show --no-dual-color since it's not the default.\n"},{"id":"356348","messageId":"20180823082725.GB160081@aiede.svl.corp.google.com","threadId":"49197","inReplyTo":"87d0u9ybhk.fsf@kyleam.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-23T08:27:25Z","receivedAt":"2018-08-23T08:27:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kyle Meyer wrote:\n> Jonathan Nieder writes:\n\n>> I think what you mean is something like \"color only based on the\n>> diff-between-diffs\".\n>\n> Yeah, I that sounds OK to me.\n\nThanks. Want to prepare a patch along those lines to finish this off?\n\nI'm off to sleep now, can look at this again tomorrow morning.\n\nThanks,\nJonathan\n"},{"id":"356358","messageId":"20180823120026.32127-1-kyle@kyleam.com","threadId":"49197","inReplyTo":"20180823082725.GB160081@aiede.svl.corp.google.com","subject":"[PATCH v2] range-diff: update stale summary of --no-dual-color","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2018-08-23T12:00:26Z","receivedAt":"2018-08-23T12:00:53Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"275267937b (range-diff: make dual-color the default mode, 2018-08-13)\nreplaced --dual-color with --no-dual-color but left the option's\nsummary untouched. Rewrite the summary to describe --no-dual-color\nrather than dual-color.\n\nHelped-by: Jonathan Nieder \nSigned-off-by: Kyle Meyer \n---\n builtin/range-diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex f52d45d9d6..057afdbf46 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"Percentage by which creation is weighted\")),\n \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n-\t\t\t N_(\"color both diff and diff-between-diffs\")),\n+\t\t\t N_(\"color only based on the diff-between-diffs\")),\n \t\tOPT_END()\n \t};\n \tint i, j, res = 0;\n-- \n2.18.0\n\n"},{"id":"356365","messageId":"nycvar.QRO.7.76.6.1808231629540.73@tvgsbejvaqbjf.bet","threadId":"49197","inReplyTo":"20180823032238.GI92374@aiede.svl.corp.google.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-23T14:31:15Z","receivedAt":"2018-08-23T14:31:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Wed, 22 Aug 2018, Jonathan Nieder wrote:\n\n> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n> \t\t\t N_(\"Percentage by which creation is weighted\")),\n> -\t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n> +\t\tOPT_BOOL(0, \"dual-color\", &dual_color,\n> +\t\t\t N_(\"color both diff and diff-between-diffs (default)\")),\n\nThere is one very good reason *not* to do that. And that reason is the\noutput of `git range-diff -h`. If anybody read that the option\n`--dual-color` exists, they are prone to believe that the default is *not*\ndual color. In contrast, when reading `--no-dual-color`, it is clear that\ndual color mode is the default.\n\nCiao,\nDscho\n\n> \t\tOPT_END()\n> \t};\n> \tint i, j, res = 0;\n> @@ -63,8 +63,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n> \t\t\t options + ARRAY_SIZE(options) - 1, /* OPT_END */\n> \t\t\t builtin_range_diff_usage, 0);\n> \n> -\tif (simple_color < 1) {\n> -\t\tif (!simple_color)\n> +\tif (dual_color != 0) {\n> +\t\tif (dual_color > 0)\n> \t\t\t/* force color when --dual-color was used */\n> \t\t\tdiffopt.use_color = 1;\n> \t\tdiffopt.flags.dual_color_diffed_diffs = 1;\n> \n"},{"id":"356405","messageId":"nycvar.QRO.7.76.6.1808232254260.73@tvgsbejvaqbjf.bet","threadId":"49197","inReplyTo":"20180823023955.12980-1-kyle@kyleam.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-23T20:54:44Z","receivedAt":"2018-08-23T20:54:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Kyle,\n\nOn Wed, 22 Aug 2018, Kyle Meyer wrote:\n\n> 275267937b (range-diff: make dual-color the default mode, 2018-08-13)\n> replaced --dual-color with --no-dual-color but left the option's\n> summary untouched. Rewrite the summary to describe --no-dual-color\n> rather than dual-color.\n> \n> Signed-off-by: Kyle Meyer \n> ---\n> builtin/range-diff.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n> index f52d45d9d6..7dc90a5ec3 100644\n> --- a/builtin/range-diff.c\n> +++ b/builtin/range-diff.c\n> @@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n> \t\t\t N_(\"Percentage by which creation is weighted\")),\n> \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n> +\t\t\t N_(\"restrict coloring to outer diff markers\")),\n\nHow about \"use simple diff colors\" instead?\n\nCiao,\nJohannes\n"},{"id":"356411","messageId":"20180823211028.GA99542@aiede.svl.corp.google.com","threadId":"49197","inReplyTo":"20180823120026.32127-1-kyle@kyleam.com","subject":"Re: [PATCH v2] range-diff: update stale summary of --no-dual-color","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-23T21:10:28Z","receivedAt":"2018-08-23T21:10:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kyle Meyer wrote:\n\n> \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n> +\t\t\t N_(\"color only based on the diff-between-diffs\")),\n\nReviewed-by: Jonathan Nieder \n\nDscho's suggestion \"use simple diff colors\" also sounds fine to me\n(probably even better).\n\nThanks,\nJonathan\n"},{"id":"356412","messageId":"20180823211214.GB99542@aiede.svl.corp.google.com","threadId":"49197","inReplyTo":"nycvar.QRO.7.76.6.1808231629540.73@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-23T21:12:14Z","receivedAt":"2018-08-23T21:12:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJohannes Schindelin wrote:\n> On Wed, 22 Aug 2018, Jonathan Nieder wrote:\n\n>> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n>> \t\t\t N_(\"Percentage by which creation is weighted\")),\n>> -\t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n>> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n>> +\t\tOPT_BOOL(0, \"dual-color\", &dual_color,\n>> +\t\t\t N_(\"color both diff and diff-between-diffs (default)\")),\n>\n> There is one very good reason *not* to do that. And that reason is the\n> output of `git range-diff -h`. If anybody read that the option\n> `--dual-color` exists, they are prone to believe that the default is *not*\n> dual color. In contrast, when reading `--no-dual-color`, it is clear that\n> dual color mode is the default.\n\nThe whole patch is about \"git range-diff -h\" output, and of course I\ntested it. Did you see the \"(default)\" part of the string in the\npatch?\n\nThat said, the conversation continued and I agree with the conclusion\nit led to (which is better than the patch you're replying to).\n\nThanks,\nJonathan\n"},{"id":"356413","messageId":"xmqq1saoixxs.fsf@gitster-ct.c.googlers.com","threadId":"49197","inReplyTo":"nycvar.QRO.7.76.6.1808232254260.73@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-23T21:18:39Z","receivedAt":"2018-08-23T21:18:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Wed, 22 Aug 2018, Kyle Meyer wrote:\n>\n>> 275267937b (range-diff: make dual-color the default mode, 2018-08-13)\n>> replaced --dual-color with --no-dual-color but left the option's\n>> summary untouched. Rewrite the summary to describe --no-dual-color\n>> rather than dual-color.\n>> \n>> Signed-off-by: Kyle Meyer \n>> ---\n>> builtin/range-diff.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>> \n>> diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n>> index f52d45d9d6..7dc90a5ec3 100644\n>> --- a/builtin/range-diff.c\n>> +++ b/builtin/range-diff.c\n>> @@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n>> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n>> \t\t\t N_(\"Percentage by which creation is weighted\")),\n>> \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n>> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n>> +\t\t\t N_(\"restrict coloring to outer diff markers\")),\n>\n> How about \"use simple diff colors\" instead?\n\nI am wondering if it makes sense to remove the option altogether.\nI've been trying to view the comparison of the same ranges in both\nstyles for the past few days, and I never found a reason to choose\n\"no dual color\" option myself.\n"},{"id":"356415","messageId":"87pny8wz6z.fsf@kyleam.com","threadId":"49197","inReplyTo":"xmqq1saoixxs.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2018-08-23T21:27:48Z","receivedAt":"2018-08-23T21:27:59Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n\n[...]\n\n>>> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n>>> +\t\t\t N_(\"restrict coloring to outer diff markers\")),\n>>\n>> How about \"use simple diff colors\" instead?\n\nThat's certainly better than the one above, and I also prefer it to\n\"color only based on the diff-between-diffs\" in v2.\n\n> I am wondering if it makes sense to remove the option altogether.\n> I've been trying to view the comparison of the same ranges in both\n> styles for the past few days, and I never found a reason to choose\n> \"no dual color\" option myself.\n\nBut I like this suggestion even better.\n"},{"id":"356417","messageId":"nycvar.QRO.7.76.6.1808232333160.73@tvgsbejvaqbjf.bet","threadId":"49197","inReplyTo":"xmqq1saoixxs.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-23T21:41:04Z","receivedAt":"2018-08-23T21:41:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 23 Aug 2018, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Wed, 22 Aug 2018, Kyle Meyer wrote:\n> >\n> >> 275267937b (range-diff: make dual-color the default mode, 2018-08-13)\n> >> replaced --dual-color with --no-dual-color but left the option's\n> >> summary untouched. Rewrite the summary to describe --no-dual-color\n> >> rather than dual-color.\n> >> \n> >> Signed-off-by: Kyle Meyer \n> >> ---\n> >> builtin/range-diff.c | 2 +-\n> >> 1 file changed, 1 insertion(+), 1 deletion(-)\n> >> \n> >> diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n> >> index f52d45d9d6..7dc90a5ec3 100644\n> >> --- a/builtin/range-diff.c\n> >> +++ b/builtin/range-diff.c\n> >> @@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n> >> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n> >> \t\t\t N_(\"Percentage by which creation is weighted\")),\n> >> \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n> >> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n> >> +\t\t\t N_(\"restrict coloring to outer diff markers\")),\n> >\n> > How about \"use simple diff colors\" instead?\n> \n> I am wondering if it makes sense to remove the option altogether.\n> I've been trying to view the comparison of the same ranges in both\n> styles for the past few days, and I never found a reason to choose\n> \"no dual color\" option myself.\n\nWe do have a track record of making decisions based on our little bubble,\ndon't we.\n\nOn IRC, there is at least on publicly viewable comment by a user who\npreferred the simple color diff, at least in one use case:\n\nhttp://colabti.org/irclogger/irclogger_log/git-devel?date=2018-07-13#l97\n\nAnd I am living in my own bubble, too. I think I heard feedback regarding\nrange-diff from some dozen people. Multiplying 6% by the download numbers\nof Git for Windows alone... that's a lot of people who can put\n--no-dual-color to good use at least in *some* situations.\n\nIn short: I am hesitant to remove a feature that would help some users.\n\nCiao,\nDscho\n"},{"id":"356418","messageId":"xmqqsh34hhzf.fsf@gitster-ct.c.googlers.com","threadId":"49197","inReplyTo":"87pny8wz6z.fsf@kyleam.com","subject":"Re: [PATCH] range-diff: update stale summary of --no-dual-color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-23T21:48:36Z","receivedAt":"2018-08-23T21:48:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Meyer writes:\n\n> Junio C Hamano writes:\n>\n>> Johannes Schindelin writes:\n>\n> [...]\n>\n>>>> -\t\t\t N_(\"color both diff and diff-between-diffs\")),\n>>>> +\t\t\t N_(\"restrict coloring to outer diff markers\")),\n>>>\n>>> How about \"use simple diff colors\" instead?\n>\n> That's certainly better than the one above, and I also prefer it to\n> \"color only based on the diff-between-diffs\" in v2.\n>\n>> I am wondering if it makes sense to remove the option altogether.\n>> I've been trying to view the comparison of the same ranges in both\n>> styles for the past few days, and I never found a reason to choose\n>> \"no dual color\" option myself.\n>\n> But I like this suggestion even better.\n\nThat wasn't even a suggestion. I just did not see the point of the\noptional behaviour myself, and was soliciting concrete examples\nwhere the --no-dual mode would help.\n\n"},{"id":"356419","messageId":"20180823215748.3474-1-kyle@kyleam.com","threadId":"49197","inReplyTo":"20180823211028.GA99542@aiede.svl.corp.google.com","subject":"[PATCH v3] range-diff: update stale summary of --no-dual-color","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2018-08-23T21:57:48Z","receivedAt":"2018-08-23T21:57:58Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"275267937b (range-diff: make dual-color the default mode, 2018-08-13)\nreplaced --dual-color with --no-dual-color but left the option's\nsummary untouched. Rewrite the summary to describe --no-dual-color\nrather than dual-color.\n\nHelped-by: Jonathan Nieder \nHelped-by: Johannes Schindelin \nSigned-off-by: Kyle Meyer \n---\n builtin/range-diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex f52d45d9d6..0aa9bed41f 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"Percentage by which creation is weighted\")),\n \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n-\t\t\t N_(\"color both diff and diff-between-diffs\")),\n+\t\t\t N_(\"use simple diff colors\")),\n \t\tOPT_END()\n \t};\n \tint i, j, res = 0;\n-- \n2.18.0\n\n"},{"id":"356420","messageId":"20180823220207.GC99542@aiede.svl.corp.google.com","threadId":"49197","inReplyTo":"20180823215748.3474-1-kyle@kyleam.com","subject":"Re: [PATCH v3] range-diff: update stale summary of --no-dual-color","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-23T22:02:07Z","receivedAt":"2018-08-23T22:02:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kyle Meyer wrote:\n\n> 275267937b (range-diff: make dual-color the default mode, 2018-08-13)\n> replaced --dual-color with --no-dual-color but left the option's\n> summary untouched. Rewrite the summary to describe --no-dual-color\n> rather than dual-color.\n>\n> Helped-by: Jonathan Nieder \n\nHa, I don't think I deserve much credit here, but I'll take it. ;-)\n\n> Helped-by: Johannes Schindelin \n> Signed-off-by: Kyle Meyer \n> ---\n> builtin/range-diff.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nReviewed-by: Jonathan Nieder \n"},{"id":"356421","messageId":"xmqqo9dshh1j.fsf@gitster-ct.c.googlers.com","threadId":"49197","inReplyTo":"20180823220207.GC99542@aiede.svl.corp.google.com","subject":"Re: [PATCH v3] range-diff: update stale summary of --no-dual-color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-23T22:08:56Z","receivedAt":"2018-08-23T22:09:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Kyle Meyer wrote:\n>\n>> 275267937b (range-diff: make dual-color the default mode, 2018-08-13)\n>> replaced --dual-color with --no-dual-color but left the option's\n>> summary untouched. Rewrite the summary to describe --no-dual-color\n>> rather than dual-color.\n>>\n>> Helped-by: Jonathan Nieder \n>\n> Ha, I don't think I deserve much credit here, but I'll take it. ;-)\n>\n>> Helped-by: Johannes Schindelin \n>> Signed-off-by: Kyle Meyer \n>> ---\n>> builtin/range-diff.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> Reviewed-by: Jonathan Nieder \n\nSorry, too late. I'll revert the merge of the previous round out of\n'next' and requeue this one, but that will have to wait until the\nnext integration cycle.\n"},{"id":"356422","messageId":"20180823221152.GD99542@aiede.svl.corp.google.com","threadId":"49197","inReplyTo":"xmqqo9dshh1j.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] range-diff: update stale summary of --no-dual-color","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-23T22:11:52Z","receivedAt":"2018-08-23T22:11:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>> Kyle Meyer wrote:\n\n>>> Subject: [PATCH v3] range-diff: update stale summary of --no-dual-color\n[...]\n>> Reviewed-by: Jonathan Nieder \n>\n> Sorry, too late. I'll revert the merge of the previous round out of\n> 'next' and requeue this one, but that will have to wait until the\n> next integration cycle.\n\nThanks for the heads up. Sounds like a fine plan.\n\nJonathan\n"},{"id":"356597","messageId":"xmqq5zzvlmjy.fsf@gitster-ct.c.googlers.com","threadId":"49197","inReplyTo":"20180823221152.GD99542@aiede.svl.corp.google.com","subject":"Re: [PATCH v3] range-diff: update stale summary of --no-dual-color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-27T17:57:37Z","receivedAt":"2018-08-27T17:57:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>> Jonathan Nieder writes:\n>>> Kyle Meyer wrote:\n>\n>>>> Subject: [PATCH v3] range-diff: update stale summary of --no-dual-color\n> [...]\n>>> Reviewed-by: Jonathan Nieder \n>>\n>> Sorry, too late. I'll revert the merge of the previous round out of\n>> 'next' and requeue this one, but that will have to wait until the\n>> next integration cycle.\n>\n> Thanks for the heads up. Sounds like a fine plan.\n\nHaving said that, I do not think the change from v2 to v3 is an\nimprovement. At least the one in v2 explained what the input is to\nthe logic to determine colors, helping the users to understand what\nis painted and why and decide if that coloring is useful to them.\n\nThe phrasing in v3, \"use simple diff colors\", does not give much\ninformation over saying something like \"paint it differently\" (which\nis silly because \"differently\" is a given, once you give an option\nto cause a non-default behaviour).\n\nNot limited to this particular case, but in general, subjective\nwords like \"simple\" have much less information density than more\nspecific words, and we need to be careful when spending bits on a\nlimited space (like option description) to them.\n\n"}]}