Re: [PATCH v3 2/2] merge-recursive: more consistent interface

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 17, 2016, 01:52 UTC
Message-ID: <CAPig+cQMutC1rDoLTZHu6zYi2i7s9jYkO8Nb29q8tJwEjq+ADg@mail.gmail.com>
In-Reply-To: <1455671495-10908-3-git-send-email-felipegassis@gmail.com>

On Tue, Feb 16, 2016 at 8:11 PM, Felipe Gonçalves Assis <felipeg.assis@gmail.com> wrote:

> Add strategy option find-renames, following git-diff interface. This
> makes the option rename-threshold redundant.

Missing sign-off.

Show 11 quoted lines

> ---
> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> @@ -85,11 +85,15 @@ no-renames;;
>         Turn off rename detection.
>         See also linkgit:git-diff[1] `--no-renames`.
>
> -rename-threshold=<n>;;
> +find-renames[=<n>];;
>         Controls the similarity threshold used for rename detection.
>         Re-enables rename detection if disabled by a preceding
>         `no-renames`.

This may need some enhancement. It doesn't, for instance, talk about what bare --find-renames (without "=n") means.

Show 6 quoted lines

> -       See also linkgit:git-diff[1] `-M`.
> +       See also linkgit:git-diff[1] `--find-renames`.
> +
> +rename-threshold=<n>;;
> +       Synonym for `find-renames=<n>`. Present for historical reasons.
> +       New scripts should prefer the `find-renames=<n>` syntax.

It might be sufficient to say merely:

    Deprecated synonym for `--find-renames=<n>`.

which implies the bits about "historical reasons" and "new scripts should prefer...".

Show 20 quoted lines

>  subtree[=<path>];;
>         This option is a more advanced form of 'subtree' strategy, where
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 6dd0a11..700febd 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -2094,7 +2094,10 @@ int parse_merge_opt(struct merge_options *o, const char *s)
>                 o->renormalize = 0;
>         else if (!strcmp(s, "no-renames"))
>                 o->detect_rename = 0;
> -       else if (skip_prefix(s, "rename-threshold=", &arg)) {
> +       else if (!strcmp(s, "find-renames"))
> +               o->detect_rename = 1;
> +       else if (skip_prefix(s, "find-renames=", &arg) ||
> +                skip_prefix(s, "rename-threshold=", &arg)) {
>                 if ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)
>                         return -1;
>                 o->detect_rename = 1;
> --
> 2.7.1.288.gfad33a8