{"thread":{"id":"41429","subject":"[PATCH v3 0/2] merge-recursive: option to disable renames","startedAt":"2016-02-17T01:11:33Z","lastAt":"2016-02-17T03:38:23Z","messageCount":8,"participants":["Felipe Gonçalves Assis","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"278425","messageId":"1455671495-10908-1-git-send-email-felipegassis@gmail.com","threadId":"41429","inReplyTo":null,"subject":"[PATCH v3 0/2] merge-recursive: option to disable renames","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-17T01:11:33Z","receivedAt":"2016-02-17T01:11:33Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"No more renames option. rename-threshold enables renames.\n\ndetect_rename is now a simple boolean. Its value is no longer linked to\nDIFF_ANYTHING symbol.\n\nA second optional patch is included, teaching merge-recursive to take\n\"find-renames[=]\" as well, for consistency.\n\nFelipe Gonçalves Assis (2):\n merge-recursive: option to disable renames\n merge-recursive: more consistent interface\n\n Documentation/merge-strategies.txt | 14 ++++++++++++--\n merge-recursive.c | 12 +++++++++++-\n merge-recursive.h | 1 +\n 3 files changed, 24 insertions(+), 3 deletions(-)\n\n-- \n2.7.1.288.gfad33a8\n"},{"id":"278426","messageId":"1455671495-10908-2-git-send-email-felipegassis@gmail.com","threadId":"41429","inReplyTo":"1455671495-10908-1-git-send-email-felipegassis@gmail.com","subject":"[PATCH v3 1/2] merge-recursive: option to disable renames","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-17T01:11:34Z","receivedAt":"2016-02-17T01:11:34Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"The recursive strategy turns on rename detection by default. Add a\nstrategy option to disable rename detection even for exact renames.\n\nSigned-off-by: Felipe Gonçalves Assis \n---\n Documentation/merge-strategies.txt | 6 ++++++\n merge-recursive.c | 7 +++++++\n merge-recursive.h | 1 +\n 3 files changed, 14 insertions(+)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 7bbd19b..1a5e197 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -81,8 +81,14 @@ no-renormalize;;\n \tDisables the `renormalize` option. This overrides the\n \t`merge.renormalize` configuration variable.\n \n+no-renames;;\n+\tTurn off rename detection.\n+\tSee also linkgit:git-diff[1] `--no-renames`.\n+\n rename-threshold=;;\n \tControls the similarity threshold used for rename detection.\n+\tRe-enables rename detection if disabled by a preceding\n+\t`no-renames`.\n \tSee also linkgit:git-diff[1] `-M`.\n \n subtree[=];;\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 8eabde2..6dd0a11 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -482,6 +482,9 @@ static struct string_list *get_renames(struct merge_options *o,\n \tstruct diff_options opts;\n \n \trenames = xcalloc(1, sizeof(struct string_list));\n+\tif (!o->detect_rename)\n+\t\treturn renames;\n+\n \tdiff_setup(&opts);\n \tDIFF_OPT_SET(&opts, RECURSIVE);\n \tDIFF_OPT_CLR(&opts, RENAME_EMPTY);\n@@ -2039,6 +2042,7 @@ void init_merge_options(struct merge_options *o)\n \to->diff_rename_limit = -1;\n \to->merge_rename_limit = -1;\n \to->renormalize = 0;\n+\to->detect_rename = 1;\n \tmerge_recursive_config(o);\n \tif (getenv(\"GIT_MERGE_VERBOSITY\"))\n \t\to->verbosity =\n@@ -2088,9 +2092,12 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\to->renormalize = 1;\n \telse if (!strcmp(s, \"no-renormalize\"))\n \t\to->renormalize = 0;\n+\telse if (!strcmp(s, \"no-renames\"))\n+\t\to->detect_rename = 0;\n \telse if (skip_prefix(s, \"rename-threshold=\", &arg)) {\n \t\tif ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)\n \t\t\treturn -1;\n+\t\to->detect_rename = 1;\n \t}\n \telse\n \t\treturn -1;\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 9e090a3..52f0201 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -17,6 +17,7 @@ struct merge_options {\n \tunsigned renormalize : 1;\n \tlong xdl_opts;\n \tint verbosity;\n+\tint detect_rename;\n \tint diff_rename_limit;\n \tint merge_rename_limit;\n \tint rename_score;\n-- \n2.7.1.288.gfad33a8\n"},{"id":"278427","messageId":"1455671495-10908-3-git-send-email-felipegassis@gmail.com","threadId":"41429","inReplyTo":"1455671495-10908-1-git-send-email-felipegassis@gmail.com","subject":"[PATCH v3 2/2] merge-recursive: more consistent interface","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-17T01:11:35Z","receivedAt":"2016-02-17T01:11:35Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"Add strategy option find-renames, following git-diff interface. This\nmakes the option rename-threshold redundant.\n---\n\nA minor note\ngit diff --check complains about an indent with spaces here, but I think I did\nthe right thing: indented with tabs and aligned with spaces. If desired, I can\nalign with tabs to avoid this.\n\n Documentation/merge-strategies.txt | 8 ++++++--\n merge-recursive.c | 5 ++++-\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 1a5e197..f8618c9 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -85,11 +85,15 @@ no-renames;;\n \tTurn off rename detection.\n \tSee also linkgit:git-diff[1] `--no-renames`.\n \n-rename-threshold=;;\n+find-renames[=];;\n \tControls the similarity threshold used for rename detection.\n \tRe-enables rename detection if disabled by a preceding\n \t`no-renames`.\n-\tSee also linkgit:git-diff[1] `-M`.\n+\tSee also linkgit:git-diff[1] `--find-renames`.\n+\n+rename-threshold=;;\n+\tSynonym for `find-renames=`. Present for historical reasons.\n+\tNew scripts should prefer the `find-renames=` syntax.\n \n subtree[=];;\n \tThis option is a more advanced form of 'subtree' strategy, where\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 6dd0a11..700febd 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -2094,7 +2094,10 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\to->renormalize = 0;\n \telse if (!strcmp(s, \"no-renames\"))\n \t\to->detect_rename = 0;\n-\telse if (skip_prefix(s, \"rename-threshold=\", &arg)) {\n+\telse if (!strcmp(s, \"find-renames\"))\n+\t\to->detect_rename = 1;\n+\telse if (skip_prefix(s, \"find-renames=\", &arg) ||\n+\t skip_prefix(s, \"rename-threshold=\", &arg)) {\n \t\tif ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)\n \t\t\treturn -1;\n \t\to->detect_rename = 1;\n-- \n2.7.1.288.gfad33a8\n"},{"id":"278428","messageId":"CAPig+cRhbCRJhrOFcjY2avFZ0rQgCP-JT-6RTCiihOQB8nWeMw@mail.gmail.com","threadId":"41429","inReplyTo":"1455671495-10908-1-git-send-email-felipegassis@gmail.com","subject":"Re: [PATCH v3 0/2] merge-recursive: option to disable renames","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-17T01:37:55Z","receivedAt":"2016-02-17T01:37:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Feb 16, 2016 at 8:11 PM, Felipe Gonçalves Assis\n wrote:\n> No more renames option. rename-threshold enables renames.\n\nCan you add some tests? Off the top of my head, I'd expect to see at\nleast three new tests:\n\n1. --no-renames works as expected\n2. last wins in \"--no-renames --rename-threshold=x\"\n3. last wins in \"--rename-threshold=x --no-renames\"\n\n> A second optional patch is included, teaching merge-recursive to take\n> \"find-renames[=]\" as well, for consistency.\n\nTests should also accompany this change. A couple obvious ones:\n\n1. --find-rename=x works as synonym for --rename-threshold=x\n2. --fine-rename (without \"=x\") works as expected\n\nThanks.\n\n> Felipe Gonçalves Assis (2):\n> merge-recursive: option to disable renames\n> merge-recursive: more consistent interface\n>\n> Documentation/merge-strategies.txt | 14 ++++++++++++--\n> merge-recursive.c | 12 +++++++++++-\n> merge-recursive.h | 1 +\n> 3 files changed, 24 insertions(+), 3 deletions(-)\n>\n> --\n> 2.7.1.288.gfad33a8\n"},{"id":"278429","messageId":"CAPig+cRCKQ8Vpr11XB-MSNsjDXjMHstaEAz333nMZxuKC8xmEg@mail.gmail.com","threadId":"41429","inReplyTo":"1455671495-10908-2-git-send-email-felipegassis@gmail.com","subject":"Re: [PATCH v3 1/2] merge-recursive: option to disable renames","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-17T01:41:25Z","receivedAt":"2016-02-17T01:41:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Feb 16, 2016 at 8:11 PM, Felipe Gonçalves Assis\n wrote:\n> The recursive strategy turns on rename detection by default. Add a\n> strategy option to disable rename detection even for exact renames.\n>\n> Signed-off-by: Felipe Gonçalves Assis \n> ---\n> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\n> @@ -81,8 +81,14 @@ no-renormalize;;\n> +no-renames;;\n> + Turn off rename detection.\n> + See also linkgit:git-diff[1] `--no-renames`.\n> +\n> rename-threshold=;;\n> Controls the similarity threshold used for rename detection.\n> + Re-enables rename detection if disabled by a preceding\n> + `no-renames`.\n\nI'm not sure that it is necessary to mention the \"last one wins\" rule\nhere, but if you do so, does --no-renames documentation deserve\nsimilar treatment?\n\n> See also linkgit:git-diff[1] `-M`.\n"},{"id":"278430","messageId":"CAPig+cQMutC1rDoLTZHu6zYi2i7s9jYkO8Nb29q8tJwEjq+ADg@mail.gmail.com","threadId":"41429","inReplyTo":"1455671495-10908-3-git-send-email-felipegassis@gmail.com","subject":"Re: [PATCH v3 2/2] merge-recursive: more consistent interface","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-17T01:52:15Z","receivedAt":"2016-02-17T01:52:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Feb 16, 2016 at 8:11 PM, Felipe Gonçalves Assis\n wrote:\n> Add strategy option find-renames, following git-diff interface. This\n> makes the option rename-threshold redundant.\n\nMissing sign-off.\n\n> ---\n> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\n> @@ -85,11 +85,15 @@ no-renames;;\n> Turn off rename detection.\n> See also linkgit:git-diff[1] `--no-renames`.\n>\n> -rename-threshold=;;\n> +find-renames[=];;\n> Controls the similarity threshold used for rename detection.\n> Re-enables rename detection if disabled by a preceding\n> `no-renames`.\n\nThis may need some enhancement. It doesn't, for instance, talk about\nwhat bare --find-renames (without \"=n\") means.\n\n> - See also linkgit:git-diff[1] `-M`.\n> + See also linkgit:git-diff[1] `--find-renames`.\n> +\n> +rename-threshold=;;\n> + Synonym for `find-renames=`. Present for historical reasons.\n> + New scripts should prefer the `find-renames=` syntax.\n\nIt might be sufficient to say merely:\n\n Deprecated synonym for `--find-renames=`.\n\nwhich implies the bits about \"historical reasons\" and \"new scripts\nshould prefer...\".\n\n> subtree[=];;\n> This option is a more advanced form of 'subtree' strategy, where\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 6dd0a11..700febd 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -2094,7 +2094,10 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n> o->renormalize = 0;\n> else if (!strcmp(s, \"no-renames\"))\n> o->detect_rename = 0;\n> - else if (skip_prefix(s, \"rename-threshold=\", &arg)) {\n> + else if (!strcmp(s, \"find-renames\"))\n> + o->detect_rename = 1;\n> + else if (skip_prefix(s, \"find-renames=\", &arg) ||\n> + skip_prefix(s, \"rename-threshold=\", &arg)) {\n> if ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)\n> return -1;\n> o->detect_rename = 1;\n> --\n> 2.7.1.288.gfad33a8\n"},{"id":"278432","messageId":"CALMa68peGr3TsS8bTpsG3Xt5U5VtbLhKryhOSLEYaNZyjRuSgQ@mail.gmail.com","threadId":"41429","inReplyTo":"CAPig+cRCKQ8Vpr11XB-MSNsjDXjMHstaEAz333nMZxuKC8xmEg@mail.gmail.com","subject":"Re: [PATCH v3 1/2] merge-recursive: option to disable renames","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-17T03:16:12Z","receivedAt":"2016-02-17T03:16:12Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"On 16 February 2016 at 23:41, Eric Sunshine wrote:\n> On Tue, Feb 16, 2016 at 8:11 PM, Felipe Gonçalves Assis\n> wrote:\n>> The recursive strategy turns on rename detection by default. Add a\n>> strategy option to disable rename detection even for exact renames.\n>>\n>> Signed-off-by: Felipe Gonçalves Assis \n>> ---\n>> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\n>> @@ -81,8 +81,14 @@ no-renormalize;;\n>> +no-renames;;\n>> + Turn off rename detection.\n>> + See also linkgit:git-diff[1] `--no-renames`.\n>> +\n>> rename-threshold=;;\n>> Controls the similarity threshold used for rename detection.\n>> + Re-enables rename detection if disabled by a preceding\n>> + `no-renames`.\n>\n> I'm not sure that it is necessary to mention the \"last one wins\" rule\n> here, but if you do so, does --no-renames documentation deserve\n> similar treatment?\n>\n\nIt is not so much about the \"last one wins\" rule, but about the fact\nthat, given the sub-optimal name, it is not obvious that this option\neven enables renaming. The wording is just the best way I found to\nexplain that without creating additional confusion.\n\nI will apply your suggestion on the second commit. Please see the next\npatch proposal and tell what you think.\n\nThanks,\nFelipe\n"},{"id":"278437","messageId":"CALMa68oy2i6GeiUS1MLodAJ__TsqDzbRWcFVY2xdZKcoONKvzA@mail.gmail.com","threadId":"41429","inReplyTo":"CAPig+cRhbCRJhrOFcjY2avFZ0rQgCP-JT-6RTCiihOQB8nWeMw@mail.gmail.com","subject":"Re: [PATCH v3 0/2] merge-recursive: option to disable renames","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-17T03:38:23Z","receivedAt":"2016-02-17T03:38:23Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"On 16 February 2016 at 23:37, Eric Sunshine wrote:\n> On Tue, Feb 16, 2016 at 8:11 PM, Felipe Gonçalves Assis\n> wrote:\n>> No more renames option. rename-threshold enables renames.\n>\n> Can you add some tests? Off the top of my head, I'd expect to see at\n> least three new tests:\n>\n> 1. --no-renames works as expected\n> 2. last wins in \"--no-renames --rename-threshold=x\"\n> 3. last wins in \"--rename-threshold=x --no-renames\"\n>\n\nSure.\n\nJust a heads up: For the next three days I will have very little time,\nso I will start tackling this on the weekend. I submitted a fourth\nversion of the patch without the tests, so that this part of the work\ncan proceed in parallel.\n\nThanks,\nFelipe\n"}]}