{"thread":{"id":"25193","subject":"[WIP/PATCH] merge-recursive: option to specify rename threshold","startedAt":"2010-09-22T06:03:39Z","lastAt":"2010-09-28T21:15:27Z","messageCount":18,"participants":["Kevin Ballard","Junio C Hamano","Jonathan Nieder","Thell Fowler"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"151276","messageId":"1285135419-7503-1-git-send-email-kevin@sb.org","threadId":"25193","inReplyTo":null,"subject":"[WIP/PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-22T06:03:39Z","receivedAt":"2010-09-22T06:03:39Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"The recursive merge strategy turns on rename detection but leaves the\nrename score at the default. Add a strategy option to allow the user\nto specify a rename score to use.\n---\nThe only thing I'm concerned about in this patch is the duplicated\nparse_num() function. I'm inclined to take that function in diff.c,\nrename it to something like parse_rename_score(), and declare it\nin diff.h.\n\n merge-recursive.c |   43 +++++++++++++++++++++++++++++++++++++++++++\n merge-recursive.h |    1 +\n 2 files changed, 44 insertions(+), 0 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex bf611ae..f8ff30e 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -334,6 +334,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \topts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :\n \t\t\t    o->diff_rename_limit >= 0 ? o->diff_rename_limit :\n \t\t\t    500;\n+\topts.rename_score = o->rename_score;\n \topts.warn_on_too_large_rename = 1;\n \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opts) < 0)\n@@ -1552,6 +1553,43 @@ void init_merge_options(struct merge_options *o)\n \to->current_directory_set.strdup_strings = 1;\n }\n \n+// XXX: copied from diff.c\n+static int parse_num(const char **cp_p)\n+{\n+\tunsigned long num, scale;\n+\tint ch, dot;\n+\tconst char *cp = *cp_p;\n+\n+\tnum = 0;\n+\tscale = 1;\n+\tdot = 0;\n+\tfor (;;) {\n+\t\tch = *cp;\n+\t\tif ( !dot && ch == '.' ) {\n+\t\t\tscale = 1;\n+\t\t\tdot = 1;\n+\t\t} else if ( ch == '%' ) {\n+\t\t\tscale = dot ? scale*100 : 100;\n+\t\t\tcp++;\t/* % is always at the end */\n+\t\t\tbreak;\n+\t\t} else if ( ch >= '0' && ch <= '9' ) {\n+\t\t\tif ( scale < 100000 ) {\n+\t\t\t\tscale *= 10;\n+\t\t\t\tnum = (num*10) + (ch-'0');\n+\t\t\t}\n+\t\t} else {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcp++;\n+\t}\n+\t*cp_p = cp;\n+\n+\t/* user says num divided by scale and we say internally that\n+\t * is MAX_SCORE * num / scale.\n+\t */\n+\treturn (int)((num >= scale) ? MAX_SCORE : (MAX_SCORE * num / scale));\n+}\n+\n int parse_merge_opt(struct merge_options *o, const char *s)\n {\n \tif (!s || !*s)\n@@ -1576,6 +1614,11 @@ 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 (!prefixcmp(s, \"rename-score=\")) {\n+\t\tconst char *score = s + strlen(\"rename-score=\");\n+\t\tif ((o->rename_score = parse_num(&score)) == -1 || *score != 0)\n+\t\t\treturn -1;\n+\t}\n \telse\n \t\treturn -1;\n \treturn 0;\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 2eb5d1a..c8135b0 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -19,6 +19,7 @@ struct merge_options {\n \tint verbosity;\n \tint diff_rename_limit;\n \tint merge_rename_limit;\n+\tint rename_score;\n \tint call_depth;\n \tstruct strbuf obuf;\n \tstruct string_list current_file_set;\n-- \n1.7.3.237.ge0a7\n"},{"id":"151332","messageId":"1285201962-46346-1-git-send-email-kevin@sb.org","threadId":"25193","inReplyTo":"1285135419-7503-1-git-send-email-kevin@sb.org","subject":"[PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-23T00:32:42Z","receivedAt":"2010-09-23T00:32:42Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"The recursive merge strategy turns on rename detection but leaves the\nrename score at the default. Add a strategy option to allow the user\nto specify a rename score to use.\n\nSigned-off-by: Kevin Ballard <kevin@sb.org>\n---\nAs near as I can tell, there are no tests that deal with rename score.\nGiven this, I did not attempt to construct my own, as I fear such a test\nwould be far more complicated than the change itself.\n\n Documentation/merge-strategies.txt |    4 ++++\n diff.c                             |    6 +++---\n diff.h                             |    2 ++\n merge-recursive.c                  |    6 ++++++\n merge-recursive.h                  |    1 +\n 5 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 91faba5..05eb8f8 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -74,6 +74,10 @@ no-renormalize;;\n \tDisables the `renormalize` option.  This overrides the\n \t`merge.renormalize` configuration variable.\n \n+rename-score=<n>;;\n+\tControls the similarity threshold used for rename detection.\n+\tSee also linkgit:git-diff[1] `-M`.\n+\n subtree[=path];;\n \tThis option is a more advanced form of 'subtree' strategy, where\n \tthe strategy makes a guess on how two trees must be shifted to\ndiff --git a/diff.c b/diff.c\nindex a7d15e5..da88704 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3323,7 +3323,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \treturn 1;\n }\n \n-static int parse_num(const char **cp_p)\n+int parse_rename_score(const char **cp_p)\n {\n \tunsigned long num, scale;\n \tint ch, dot;\n@@ -3369,7 +3369,7 @@ static int diff_scoreopt_parse(const char *opt)\n \tif (cmd != 'M' && cmd != 'C' && cmd != 'B')\n \t\treturn -1; /* that is not a -M, -C nor -B option */\n \n-\topt1 = parse_num(&opt);\n+\topt1 = parse_rename_score(&opt);\n \tif (cmd != 'B')\n \t\topt2 = 0;\n \telse {\n@@ -3379,7 +3379,7 @@ static int diff_scoreopt_parse(const char *opt)\n \t\t\treturn -1; /* we expect -B80/99 or -B80 */\n \t\telse {\n \t\t\topt++;\n-\t\t\topt2 = parse_num(&opt);\n+\t\t\topt2 = parse_rename_score(&opt);\n \t\t}\n \t}\n \tif (*opt != 0)\ndiff --git a/diff.h b/diff.h\nindex e17383c..1a263e9 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -332,4 +332,6 @@ extern void emit_line(struct diff_options *o, const char *set, const char *reset\n \n extern char *quote_two(const char *one, const char *two);\n \n+extern int parse_rename_score(const char **cp_p);\n+\n #endif /* DIFF_H */\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex bf611ae..4d131da 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -334,6 +334,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \topts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :\n \t\t\t    o->diff_rename_limit >= 0 ? o->diff_rename_limit :\n \t\t\t    500;\n+\topts.rename_score = o->rename_score;\n \topts.warn_on_too_large_rename = 1;\n \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opts) < 0)\n@@ -1576,6 +1577,11 @@ 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 (!prefixcmp(s, \"rename-score=\")) {\n+\t\tconst char *score = s + strlen(\"rename-score=\");\n+\t\tif ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0)\n+\t\t\treturn -1;\n+\t}\n \telse\n \t\treturn -1;\n \treturn 0;\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 2eb5d1a..c8135b0 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -19,6 +19,7 @@ struct merge_options {\n \tint verbosity;\n \tint diff_rename_limit;\n \tint merge_rename_limit;\n+\tint rename_score;\n \tint call_depth;\n \tstruct strbuf obuf;\n \tstruct string_list current_file_set;\n-- \n1.7.3.237.g22e9\n"},{"id":"151333","messageId":"A0604F16-CA84-4A84-B74B-CE8AB455DF77@sb.org","threadId":"25193","inReplyTo":"1285201962-46346-1-git-send-email-kevin@sb.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-23T00:38:41Z","receivedAt":"2010-09-23T00:38:41Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"Ignore this patch, I just discovered that I was still operating on pre-reset next and it doesn't apply cleanly on top of the current tip.\n\n-Kevin Ballard\n\nOn Sep 22, 2010, at 5:32 PM, Kevin Ballard wrote:\n\n> The recursive merge strategy turns on rename detection but leaves the\n> rename score at the default. Add a strategy option to allow the user\n> to specify a rename score to use.\n> \n> Signed-off-by: Kevin Ballard <kevin@sb.org>\n> ---\n> As near as I can tell, there are no tests that deal with rename score.\n> Given this, I did not attempt to construct my own, as I fear such a test\n> would be far more complicated than the change itself.\n> \n> Documentation/merge-strategies.txt |    4 ++++\n> diff.c                             |    6 +++---\n> diff.h                             |    2 ++\n> merge-recursive.c                  |    6 ++++++\n> merge-recursive.h                  |    1 +\n> 5 files changed, 16 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\n> index 91faba5..05eb8f8 100644\n> --- a/Documentation/merge-strategies.txt\n> +++ b/Documentation/merge-strategies.txt\n> @@ -74,6 +74,10 @@ no-renormalize;;\n> \tDisables the `renormalize` option.  This overrides the\n> \t`merge.renormalize` configuration variable.\n> \n> +rename-score=<n>;;\n> +\tControls the similarity threshold used for rename detection.\n> +\tSee also linkgit:git-diff[1] `-M`.\n> +\n> subtree[=path];;\n> \tThis option is a more advanced form of 'subtree' strategy, where\n> \tthe strategy makes a guess on how two trees must be shifted to\n> diff --git a/diff.c b/diff.c\n> index a7d15e5..da88704 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3323,7 +3323,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n> \treturn 1;\n> }\n> \n> -static int parse_num(const char **cp_p)\n> +int parse_rename_score(const char **cp_p)\n> {\n> \tunsigned long num, scale;\n> \tint ch, dot;\n> @@ -3369,7 +3369,7 @@ static int diff_scoreopt_parse(const char *opt)\n> \tif (cmd != 'M' && cmd != 'C' && cmd != 'B')\n> \t\treturn -1; /* that is not a -M, -C nor -B option */\n> \n> -\topt1 = parse_num(&opt);\n> +\topt1 = parse_rename_score(&opt);\n> \tif (cmd != 'B')\n> \t\topt2 = 0;\n> \telse {\n> @@ -3379,7 +3379,7 @@ static int diff_scoreopt_parse(const char *opt)\n> \t\t\treturn -1; /* we expect -B80/99 or -B80 */\n> \t\telse {\n> \t\t\topt++;\n> -\t\t\topt2 = parse_num(&opt);\n> +\t\t\topt2 = parse_rename_score(&opt);\n> \t\t}\n> \t}\n> \tif (*opt != 0)\n> diff --git a/diff.h b/diff.h\n> index e17383c..1a263e9 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -332,4 +332,6 @@ extern void emit_line(struct diff_options *o, const char *set, const char *reset\n> \n> extern char *quote_two(const char *one, const char *two);\n> \n> +extern int parse_rename_score(const char **cp_p);\n> +\n> #endif /* DIFF_H */\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index bf611ae..4d131da 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -334,6 +334,7 @@ static struct string_list *get_renames(struct merge_options *o,\n> \topts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :\n> \t\t\t    o->diff_rename_limit >= 0 ? o->diff_rename_limit :\n> \t\t\t    500;\n> +\topts.rename_score = o->rename_score;\n> \topts.warn_on_too_large_rename = 1;\n> \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n> \tif (diff_setup_done(&opts) < 0)\n> @@ -1576,6 +1577,11 @@ 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 (!prefixcmp(s, \"rename-score=\")) {\n> +\t\tconst char *score = s + strlen(\"rename-score=\");\n> +\t\tif ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0)\n> +\t\t\treturn -1;\n> +\t}\n> \telse\n> \t\treturn -1;\n> \treturn 0;\n> diff --git a/merge-recursive.h b/merge-recursive.h\n> index 2eb5d1a..c8135b0 100644\n> --- a/merge-recursive.h\n> +++ b/merge-recursive.h\n> @@ -19,6 +19,7 @@ struct merge_options {\n> \tint verbosity;\n> \tint diff_rename_limit;\n> \tint merge_rename_limit;\n> +\tint rename_score;\n> \tint call_depth;\n> \tstruct strbuf obuf;\n> \tstruct string_list current_file_set;\n> -- \n> 1.7.3.237.g22e9\n> \n"},{"id":"151334","messageId":"1285202724-52474-1-git-send-email-kevin@sb.org","threadId":"25193","inReplyTo":"A0604F16-CA84-4A84-B74B-CE8AB455DF77@sb.org","subject":"[PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-23T00:45:24Z","receivedAt":"2010-09-23T00:45:24Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"The recursive merge strategy turns on rename detection but leaves the\nrename score at the default. Add a strategy option to allow the user\nto specify a rename score to use.\n\nSigned-off-by: Kevin Ballard <kevin@sb.org>\n---\nThis patch was generated off of the tip of the next branch.\n\nAs with the previous version, there are no tests included here. There seem\nto be no tests for the rename score in general, and I was not prepared to\ntry to create my own.\n\n Documentation/merge-strategies.txt |    4 ++++\n diff.c                             |    6 +++---\n diff.h                             |    2 ++\n merge-recursive.c                  |    6 ++++++\n merge-recursive.h                  |    1 +\n 5 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 91faba5..05eb8f8 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -74,6 +74,10 @@ no-renormalize;;\n \tDisables the `renormalize` option.  This overrides the\n \t`merge.renormalize` configuration variable.\n \n+rename-score=<n>;;\n+\tControls the similarity threshold used for rename detection.\n+\tSee also linkgit:git-diff[1] `-M`.\n+\n subtree[=path];;\n \tThis option is a more advanced form of 'subtree' strategy, where\n \tthe strategy makes a guess on how two trees must be shifted to\ndiff --git a/diff.c b/diff.c\nindex cc73061..d862234 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3323,7 +3323,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \treturn 1;\n }\n \n-static int parse_num(const char **cp_p)\n+int parse_rename_score(const char **cp_p)\n {\n \tunsigned long num, scale;\n \tint ch, dot;\n@@ -3369,7 +3369,7 @@ static int diff_scoreopt_parse(const char *opt)\n \tif (cmd != 'M' && cmd != 'C' && cmd != 'B')\n \t\treturn -1; /* that is not a -M, -C nor -B option */\n \n-\topt1 = parse_num(&opt);\n+\topt1 = parse_rename_score(&opt);\n \tif (cmd != 'B')\n \t\topt2 = 0;\n \telse {\n@@ -3379,7 +3379,7 @@ static int diff_scoreopt_parse(const char *opt)\n \t\t\treturn -1; /* we expect -B80/99 or -B80 */\n \t\telse {\n \t\t\topt++;\n-\t\t\topt2 = parse_num(&opt);\n+\t\t\topt2 = parse_rename_score(&opt);\n \t\t}\n \t}\n \tif (*opt != 0)\ndiff --git a/diff.h b/diff.h\nindex 1fd44f5..0083d92 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -315,4 +315,6 @@ extern size_t fill_textconv(struct userdiff_driver *driver,\n \n extern struct userdiff_driver *get_textconv(struct diff_filespec *one);\n \n+extern int parse_rename_score(const char **cp_p);\n+\n #endif /* DIFF_H */\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 325a97b..2e3ef44 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -334,6 +334,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \topts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :\n \t\t\t    o->diff_rename_limit >= 0 ? o->diff_rename_limit :\n \t\t\t    500;\n+\topts.rename_score = o->rename_score;\n \topts.warn_on_too_large_rename = 1;\n \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opts) < 0)\n@@ -1576,6 +1577,11 @@ 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 (!prefixcmp(s, \"rename-score=\")) {\n+\t\tconst char *score = s + strlen(\"rename-score=\");\n+\t\tif ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0)\n+\t\t\treturn -1;\n+\t}\n \telse\n \t\treturn -1;\n \treturn 0;\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 2eb5d1a..c8135b0 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -19,6 +19,7 @@ struct merge_options {\n \tint verbosity;\n \tint diff_rename_limit;\n \tint merge_rename_limit;\n+\tint rename_score;\n \tint call_depth;\n \tstruct strbuf obuf;\n \tstruct string_list current_file_set;\n-- \n1.7.3.68.ge6d63\n"},{"id":"151783","messageId":"7vk4m7n7uo.fsf@alter.siamese.dyndns.org","threadId":"25193","inReplyTo":"1285202724-52474-1-git-send-email-kevin@sb.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-27T04:11:43Z","receivedAt":"2010-09-27T04:11:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Ballard <kevin@sb.org> writes:\n\n> The recursive merge strategy turns on rename detection but leaves the\n> rename score at the default. Add a strategy option to allow the user\n> to specify a rename score to use.\n\nSounds straightforward, except that Documentation/diff-options.txt seems\nto call the number associated with -M \"threshold\", not \"score\".  The title\nof the patch incidentally says threshold as well ;-)\n\nAt the end-user level, this new option to merge-recursive has exactly the\nsame meaning as existing -M given to \"diff\" family; people would probably\nwant to see it made available as a synonym to \"diff\" family as well, no?\n"},{"id":"151788","messageId":"D5046A0E-7A35-421D-856F-5278FBE02914@sb.org","threadId":"25193","inReplyTo":"7vk4m7n7uo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-27T05:04:38Z","receivedAt":"2010-09-27T05:04:38Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"On Sep 26, 2010, at 9:11 PM, Junio C Hamano wrote:\n\n> Kevin Ballard <kevin@sb.org> writes:\n> \n>> The recursive merge strategy turns on rename detection but leaves the\n>> rename score at the default. Add a strategy option to allow the user\n>> to specify a rename score to use.\n> \n> Sounds straightforward, except that Documentation/diff-options.txt seems\n> to call the number associated with -M \"threshold\", not \"score\".  The title\n> of the patch incidentally says threshold as well ;-)\n\nIt says \"threshold\" because that's how the -M switch to git-diff is documented. The merge strategy option is called \"rename-score\" partially because that's what it's called internally, and partially because it's just an easier name to remember/type. I have no objections to calling it \"rename-threshold\" if you think that's better.\n\n> At the end-user level, this new option to merge-recursive has exactly the\n> same meaning as existing -M given to \"diff\" family; people would probably\n> want to see it made available as a synonym to \"diff\" family as well, no?\n\nYou mean so you can type `git diff --rename-score=50% foo`? A reasonable suggestion, but then what do we do with -B and -C? It doesn't make much sense to give a longer name to only one of the three options. This patch was concerned with simply exposing the functionality to the merge strategy and doesn't attempt to address the problem of providing long names for this trio of options.\n\n-Kevin Ballard"},{"id":"151791","messageId":"7vocbj3gjk.fsf@alter.siamese.dyndns.org","threadId":"25193","inReplyTo":"D5046A0E-7A35-421D-856F-5278FBE02914@sb.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-27T05:24:15Z","receivedAt":"2010-09-27T05:24:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Ballard <kevin@sb.org> writes:\n\n>> At the end-user level, this new option to merge-recursive has exactly the\n>> same meaning as existing -M given to \"diff\" family; people would probably\n>> want to see it made available as a synonym to \"diff\" family as well, no?\n>\n> You mean so you can type `git diff --rename-score=50% foo`? A reasonable\n> suggestion, but then what do we do with -B and -C? It doesn't make much\n> sense to give a longer name to only one of the three options. This patch\n> was concerned with simply exposing the functionality to the merge\n> strategy and doesn't attempt to address the problem of providing long\n> names for this trio of options.\n\nI would call them --break-threshold and --copy-threshold respectively.\n\nI have been happy without long option names when we originally had only\nshort names, but some people seem to be able to be more explicit, so...\n\nWhile we are at it, would it make sense to have \"merge-recursive -M20\" as\na shorthand as well?\n"},{"id":"151792","messageId":"F6C23FD9-37C4-4C20-83E7-26A1A2FC2275@sb.org","threadId":"25193","inReplyTo":"7vocbj3gjk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-27T05:34:58Z","receivedAt":"2010-09-27T05:34:58Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"On Sep 26, 2010, at 10:24 PM, Junio C Hamano wrote:\n\n> Kevin Ballard <kevin@sb.org> writes:\n> \n>>> At the end-user level, this new option to merge-recursive has exactly the\n>>> same meaning as existing -M given to \"diff\" family; people would probably\n>>> want to see it made available as a synonym to \"diff\" family as well, no?\n>> \n>> You mean so you can type `git diff --rename-score=50% foo`? A reasonable\n>> suggestion, but then what do we do with -B and -C? It doesn't make much\n>> sense to give a longer name to only one of the three options. This patch\n>> was concerned with simply exposing the functionality to the merge\n>> strategy and doesn't attempt to address the problem of providing long\n>> names for this trio of options.\n> \n> I would call them --break-threshold and --copy-threshold respectively.\n> \n> I have been happy without long option names when we originally had only\n> short names, but some people seem to be able to be more explicit, so...\n\nFair enough. Expect that naming in the next iteration of the patch.\n\n> While we are at it, would it make sense to have \"merge-recursive -M20\" as\n> a shorthand as well?\n\nSo it would be invoked like `git merge -s recursive -X M20 foo`? Looks a bit odd to me. I can add that if you think it's worthwhile though.\n\n-Kevin Ballard"},{"id":"151793","messageId":"7vk4m73enr.fsf@alter.siamese.dyndns.org","threadId":"25193","inReplyTo":"F6C23FD9-37C4-4C20-83E7-26A1A2FC2275@sb.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-27T06:04:56Z","receivedAt":"2010-09-27T06:04:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Ballard <kevin@sb.org> writes:\n\n> On Sep 26, 2010, at 10:24 PM, Junio C Hamano wrote:\n>\n>> Kevin Ballard <kevin@sb.org> writes:\n>> \n>>>> At the end-user level, this new option to merge-recursive has exactly the\n>>>> same meaning as existing -M given to \"diff\" family; people would probably\n>>>> want to see it made available as a synonym to \"diff\" family as well, no?\n>>> \n>>> You mean so you can type `git diff --rename-score=50% foo`? A reasonable\n>>> suggestion, but then what do we do with -B and -C? It doesn't make much\n>>> sense to give a longer name to only one of the three options. This patch\n>>> was concerned with simply exposing the functionality to the merge\n>>> strategy and doesn't attempt to address the problem of providing long\n>>> names for this trio of options.\n>> \n>> I would call them --break-threshold and --copy-threshold respectively.\n>> \n>> I have been happy without long option names when we originally had only\n>> short names, but some people seem to be able to be more explicit, so...\n>\n> Fair enough. Expect that naming in the next iteration of the patch.\n>\n>> While we are at it, would it make sense to have \"merge-recursive -M20\" as\n>> a shorthand as well?\n>\n> So it would be invoked like `git merge -s recursive -X M20 foo`? Looks a bit odd to me. I can add that if you think it's worthwhile though.\n\nI agree it looks odd, too ;-)\n"},{"id":"151856","messageId":"FFDB2371-6C96-472C-A650-412546636450@sb.org","threadId":"25193","inReplyTo":"7vocbj3gjk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-27T22:01:25Z","receivedAt":"2010-09-27T22:01:25Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"On Sep 26, 2010, at 10:24 PM, Junio C Hamano wrote:\n\n> Kevin Ballard <kevin@sb.org> writes:\n> \n>>> At the end-user level, this new option to merge-recursive has exactly the\n>>> same meaning as existing -M given to \"diff\" family; people would probably\n>>> want to see it made available as a synonym to \"diff\" family as well, no?\n>> \n>> You mean so you can type `git diff --rename-score=50% foo`? A reasonable\n>> suggestion, but then what do we do with -B and -C? It doesn't make much\n>> sense to give a longer name to only one of the three options. This patch\n>> was concerned with simply exposing the functionality to the merge\n>> strategy and doesn't attempt to address the problem of providing long\n>> names for this trio of options.\n> \n> I would call them --break-threshold and --copy-threshold respectively.\n> \n> I have been happy without long option names when we originally had only\n> short names, but some people seem to be able to be more explicit, so...\n\nAfter taking a look at this, it raises another question. -B, -M, and -C all have optional arguments, but the long-form names don't seem to support that. `git diff --rename-threshold= foo` would work, but looks mighty odd, and if I make it support `git diff --rename-threshold foo` that would also work, but the name doesn't seem appropriate without the argument. Should I go ahead and support `git diff --rename-threshold foo` and just live with it looking weird, or do you have a better suggestion?\n\n-Kevin Ballard"},{"id":"151865","messageId":"20100927235355.GG11957@burratino","threadId":"25193","inReplyTo":"FFDB2371-6C96-472C-A650-412546636450@sb.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-27T23:53:55Z","receivedAt":"2010-09-27T23:53:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kevin Ballard wrote:\n\n> After taking a look at this, it raises another question. -B, -M, and\n> -C all have optional arguments, but the long-form names don't seem\n> to support that.\n[...]\n>                       if I make it support `git diff\n> --rename-threshold foo` that would also work, but the name doesn't\n> seem appropriate without the argument.\n\nRight --- with merge-recursive the argument doesn't need to be\noptional, but with git diff it does.\n\nHow about\n\n\t--detect-renames=<threshold>\n\t--detect-copies=<threshold>\n\t--detect-rewrites=<threshold>/<threshold>\n\n?\n\nCiao,\nJonathan\n"},{"id":"151866","messageId":"1285631906-18200-1-git-send-email-kevin@sb.org","threadId":"25193","inReplyTo":"FFDB2371-6C96-472C-A650-412546636450@sb.org","subject":"[PATCHv2 1/2] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-27T23:58:25Z","receivedAt":"2010-09-27T23:58:25Z","isPatch":false,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"The recursive merge strategy turns on rename detection but leaves the\nrename threshold at the default. Add a strategy option to allow the user\nto specify a rename threshold to use.\n\nSigned-off-by: Kevin Ballard <kevin@sb.org>\n---\n Documentation/merge-strategies.txt |    4 ++++\n diff.c                             |    6 +++---\n diff.h                             |    2 ++\n merge-recursive.c                  |    6 ++++++\n merge-recursive.h                  |    1 +\n 5 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 91faba5..77f2606 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -74,6 +74,10 @@ no-renormalize;;\n \tDisables the `renormalize` option.  This overrides the\n \t`merge.renormalize` configuration variable.\n \n+rename-threshold=<n>;;\n+\tControls the similarity threshold used for rename detection.\n+\tSee also linkgit:git-diff[1] `-M`.\n+\n subtree[=path];;\n \tThis option is a more advanced form of 'subtree' strategy, where\n \tthe strategy makes a guess on how two trees must be shifted to\ndiff --git a/diff.c b/diff.c\nindex cc73061..d862234 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3323,7 +3323,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \treturn 1;\n }\n \n-static int parse_num(const char **cp_p)\n+int parse_rename_score(const char **cp_p)\n {\n \tunsigned long num, scale;\n \tint ch, dot;\n@@ -3369,7 +3369,7 @@ static int diff_scoreopt_parse(const char *opt)\n \tif (cmd != 'M' && cmd != 'C' && cmd != 'B')\n \t\treturn -1; /* that is not a -M, -C nor -B option */\n \n-\topt1 = parse_num(&opt);\n+\topt1 = parse_rename_score(&opt);\n \tif (cmd != 'B')\n \t\topt2 = 0;\n \telse {\n@@ -3379,7 +3379,7 @@ static int diff_scoreopt_parse(const char *opt)\n \t\t\treturn -1; /* we expect -B80/99 or -B80 */\n \t\telse {\n \t\t\topt++;\n-\t\t\topt2 = parse_num(&opt);\n+\t\t\topt2 = parse_rename_score(&opt);\n \t\t}\n \t}\n \tif (*opt != 0)\ndiff --git a/diff.h b/diff.h\nindex 1fd44f5..0083d92 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -315,4 +315,6 @@ extern size_t fill_textconv(struct userdiff_driver *driver,\n \n extern struct userdiff_driver *get_textconv(struct diff_filespec *one);\n \n+extern int parse_rename_score(const char **cp_p);\n+\n #endif /* DIFF_H */\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 325a97b..875859f 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -334,6 +334,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \topts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :\n \t\t\t    o->diff_rename_limit >= 0 ? o->diff_rename_limit :\n \t\t\t    500;\n+\topts.rename_score = o->rename_score;\n \topts.warn_on_too_large_rename = 1;\n \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opts) < 0)\n@@ -1576,6 +1577,11 @@ 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 (!prefixcmp(s, \"rename-threshold=\")) {\n+\t\tconst char *score = s + strlen(\"rename-threshold=\");\n+\t\tif ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0)\n+\t\t\treturn -1;\n+\t}\n \telse\n \t\treturn -1;\n \treturn 0;\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 2eb5d1a..c8135b0 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -19,6 +19,7 @@ struct merge_options {\n \tint verbosity;\n \tint diff_rename_limit;\n \tint merge_rename_limit;\n+\tint rename_score;\n \tint call_depth;\n \tstruct strbuf obuf;\n \tstruct string_list current_file_set;\n-- \n1.7.3.72.g8af0.dirty\n"},{"id":"151867","messageId":"1285631906-18200-2-git-send-email-kevin@sb.org","threadId":"25193","inReplyTo":"FFDB2371-6C96-472C-A650-412546636450@sb.org","subject":"[PATCHv2 2/2] diff: add synonyms for -M, -C, -B","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-27T23:58:26Z","receivedAt":"2010-09-27T23:58:26Z","isPatch":false,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"Add new long-form options --detect-renames[=<n>], --detect-copies[=<n>],\nand --break-rewrites[=[<n>][/<m>]] as synonyms for the -M, -C, and -B\noptions (respectively).\n\nSigned-off-by: Kevin Ballard <kevin@sb.org>\n---\nAfter thinking about it, I decided that --rename-threshold doesn't make sense\nas a long-form option because it doesn't make its meaning obvious when you\ndon't specify the optional argument. I also figured it doesn't need to match\nexactly with the rename-threshold merge strategy option as merge-recursive\nalready turns on rename detection and the option just controls the threshold.\nHowever, the option to git-diff actually turns on rename detection instead of\njust controlling the threshold.\n\n Documentation/diff-options.txt |    3 +++\n diff.c                         |   25 ++++++++++++++++++++++---\n 2 files changed, 25 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex f77a0f8..a511529 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -207,6 +207,7 @@ endif::git-format-patch[]\n \tdigits can be specified with `--abbrev=<n>`.\n \n -B[<n>][/<m>]::\n+--break-rewrites[=[<n>][/<m>]]::\n \tBreak complete rewrite changes into pairs of delete and\n \tcreate. This serves two purposes:\n +\n@@ -229,6 +230,7 @@ eligible for being picked up as a possible source of a rename to\n another file.\n \n -M[<n>]::\n+--detect-renames[=<n>]::\n ifndef::git-log[]\n \tDetect renames.\n endif::git-log[]\n@@ -244,6 +246,7 @@ endif::git-log[]\n \thasn't changed.\n \n -C[<n>]::\n+--detect-copies[=<n>]::\n \tDetect copies as well as renames.  See also `--find-copies-harder`.\n \tIf `n` is specified, it has the same meaning as for `-M<n>`.\n \ndiff --git a/diff.c b/diff.c\nindex d862234..d8fcf06 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3140,16 +3140,19 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\treturn stat_opt(options, av);\n \n \t/* renames options */\n-\telse if (!prefixcmp(arg, \"-B\")) {\n+\telse if (!prefixcmp(arg, \"-B\") || !prefixcmp(arg, \"--break-rewrites=\") ||\n+\t\t !strcmp(arg, \"--break-rewrites\")) {\n \t\tif ((options->break_opt = diff_scoreopt_parse(arg)) == -1)\n \t\t\treturn -1;\n \t}\n-\telse if (!prefixcmp(arg, \"-M\")) {\n+\telse if (!prefixcmp(arg, \"-M\") || !prefixcmp(arg, \"--detect-renames=\") ||\n+\t\t !strcmp(arg, \"--detect-renames\")) {\n \t\tif ((options->rename_score = diff_scoreopt_parse(arg)) == -1)\n \t\t\treturn -1;\n \t\toptions->detect_rename = DIFF_DETECT_RENAME;\n \t}\n-\telse if (!prefixcmp(arg, \"-C\")) {\n+\telse if (!prefixcmp(arg, \"-C\") || !prefixcmp(arg, \"--detect-copies=\") ||\n+\t\t !strcmp(arg, \"--detect-copies\")) {\n \t\tif (options->detect_rename == DIFF_DETECT_COPY)\n \t\t\tDIFF_OPT_SET(options, FIND_COPIES_HARDER);\n \t\tif ((options->rename_score = diff_scoreopt_parse(arg)) == -1)\n@@ -3366,6 +3369,22 @@ static int diff_scoreopt_parse(const char *opt)\n \tif (*opt++ != '-')\n \t\treturn -1;\n \tcmd = *opt++;\n+\tif (cmd == '-') {\n+\t\t/* convert the long-form arguments into short-form versions */\n+\t\tif (!prefixcmp(opt, \"break-rewrites\")) {\n+\t\t\topt += strlen(\"break-rewrites\");\n+\t\t\tif (*opt == 0 || *opt++ == '=')\n+\t\t\t\tcmd = 'B';\n+\t\t} else if (!prefixcmp(opt, \"detect-copies\")) {\n+\t\t\topt += strlen(\"detect-copies\");\n+\t\t\tif (*opt == 0 || *opt++ == '=')\n+\t\t\t\tcmd = 'C';\n+\t\t} else if (!prefixcmp(opt, \"detect-renames\")) {\n+\t\t\topt += strlen(\"detect-renames\");\n+\t\t\tif (*opt == 0 || *opt++ == '=')\n+\t\t\t\tcmd = 'M';\n+\t\t}\n+\t}\n \tif (cmd != 'M' && cmd != 'C' && cmd != 'B')\n \t\treturn -1; /* that is not a -M, -C nor -B option */\n \n-- \n1.7.3.72.g8af0.dirty\n"},{"id":"151868","messageId":"385B97D7-03F5-4698-A659-15D5D1FA939B@sb.org","threadId":"25193","inReplyTo":"20100927235355.GG11957@burratino","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-28T00:01:18Z","receivedAt":"2010-09-28T00:01:18Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"On Sep 27, 2010, at 4:53 PM, Jonathan Nieder wrote:\n\n> Kevin Ballard wrote:\n> \n>> After taking a look at this, it raises another question. -B, -M, and\n>> -C all have optional arguments, but the long-form names don't seem\n>> to support that.\n> [...]\n>>                      if I make it support `git diff\n>> --rename-threshold foo` that would also work, but the name doesn't\n>> seem appropriate without the argument.\n> \n> Right --- with merge-recursive the argument doesn't need to be\n> optional, but with git diff it does.\n> \n> How about\n> \n> \t--detect-renames=<threshold>\n> \t--detect-copies=<threshold>\n> \t--detect-rewrites=<threshold>/<threshold>\n\nGood timing, I just sent out a patch that does almost exactly this, though I went with --break-rewrites instead of --detect-rewrites.\n\n-Kevin Ballard"},{"id":"151869","messageId":"20100928000837.GH11957@burratino","threadId":"25193","inReplyTo":"385B97D7-03F5-4698-A659-15D5D1FA939B@sb.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-28T00:08:37Z","receivedAt":"2010-09-28T00:08:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kevin Ballard wrote:\n\n> Good timing, I just sent out a patch that does almost exactly this,\n> though I went with --break-rewrites instead of --detect-rewrites.\n\nBoth new patches look good to me, for what it's worth (though it\nwould be nicer to have tests, of course :)).\n"},{"id":"151870","messageId":"7FEED963-13E1-4A77-959A-FFD06669ED13@sb.org","threadId":"25193","inReplyTo":"20100928000837.GH11957@burratino","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-09-28T00:14:55Z","receivedAt":"2010-09-28T00:14:55Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"On Sep 27, 2010, at 5:08 PM, Jonathan Nieder wrote:\n\n> Kevin Ballard wrote:\n> \n>> Good timing, I just sent out a patch that does almost exactly this,\n>> though I went with --break-rewrites instead of --detect-rewrites.\n> \n> Both new patches look good to me, for what it's worth (though it\n> would be nicer to have tests, of course :)).\n\nI considered tests, and I looked at the existing ones. I am unable to find any tests that actually test setting the rename/copy score to anything other than the default, and I was a bit hesitant to add tests that simply checked to make sure the long-form option was parsed correctly, as that would just be duplicating existing tests that use the short-form arguments.\n\n-Kevin Ballard"},{"id":"151871","messageId":"20100928002442.GA2699@burratino","threadId":"25193","inReplyTo":"7FEED963-13E1-4A77-959A-FFD06669ED13@sb.org","subject":"Re: [PATCH] merge-recursive: option to specify rename threshold","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-28T00:24:42Z","receivedAt":"2010-09-28T00:24:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kevin Ballard wrote:\n\n> I looked at the existing ones. I am unable to find any tests that\n> actually test setting the rename/copy score to anything other than\n> the default\n\nYep, a quick grep shows there is none.  I think the precise meaning\nof the scores is subject to change, but a test for 1% should be\nreliable enough. :)  (Or 0%, except that that is a magic number\nwith the current code.)\n\nI can look into it tomorrow if no one else gets around to it before\nthen.\n\n'night,\nJonathan\n"},{"id":"151971","messageId":"alpine.WNT.2.00.1009281611310.6716@GWNotebook","threadId":"25193","inReplyTo":"1285631906-18200-2-git-send-email-kevin@sb.org","subject":"Re: [PATCHv2 2/2] diff: add synonyms for -M, -C, -B","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2010-09-28T21:15:27Z","receivedAt":"2010-09-28T21:15:27Z","isPatch":false,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Just wanted to throw support behind this patch as it is already helping \nour project to ease merging of a project that can't/won't modify some \nfiles that we have extensively altered.\n\nThanks for doing this Kevin!\n\nJust to note:  We are using this applied to msysgit's devel branch by \nmanually removing the diff.h chunk, applying the first patch, manually \nfixing diff.h, hen applying the second patch.  Which so far has worked \njust fine.\n\n-- \nThell\n"}]}