Re: [PATCH 1/1] wt-status: use rename settings from init_diff_ui_defaults

[Elijah Newren <newren@gmail.com>]

Hi Eckhard,

On Mon, Apr 30, 2018 at 2:34 AM, Eckhard S. Maaß
<eckhard.s.maass@googlemail.com> wrote:
> Since the very beginning, git status behaved differently for rename
> detection than other rename aware commands like git log or git show as
> it has the use of rename hard coded into it.  After 5404c116aa ("diff:
> activate diff.renames by default", 2016-02-25) the default behaves the
> same by coincidence, but a work flow like
>
>     - git add .
>     - git status
>     - git commit
>     - git show
>
> should give you the same information on renames (and/or copies if
> activated) accordingly to the diff.renames setting.

Thanks for sending this change in.  I agree with the logic.  I think
the last sentence needs a s/diff.renames setting/diff.renames and
diff.renameLimit settings/, though, because...

> With this commit the hard coded rename settings are dropped from the
> status command.
>
> Signed-off-by: Eckhard S. Maaß <eckhard.s.maass@gmail.com>
> ---
>  builtin/commit.c       |  2 +-
>  t/t4001-diff-rename.sh | 12 ++++++++++++
>  wt-status.c            |  4 ----
>  3 files changed, 13 insertions(+), 5 deletions(-)
>
<snip>
> diff --git a/wt-status.c b/wt-status.c
> index 50815e5faf..32f3bcaebd 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -625,9 +625,6 @@ static void wt_status_collect_changes_index(struct wt_status *s)
>         rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
>         rev.diffopt.format_callback = wt_status_collect_updated_cb;
>         rev.diffopt.format_callback_data = s;
> -       rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
> -       rev.diffopt.rename_limit = 200;

By removing the hard-coded value of 200, the rename limit will instead
match whatever the user specified (or, if they didn't specify
anything, then the value of 400 from diff_rename_limit_default).  I
think that's a good change, for the exact same reasons as you argue
for making diff.renames be used everywhere in the commit message, it
just needs to be mentioned in the commit message.

Everything else in the patch looks good to me, so with that change
feel free to add:

Reviewed-by: Elijah Newren <newren@gmail.com>