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

From: Matthieu Moy <git@matthieu-moy.fr>
Date: May 1, 2018, 11:09 UTC
Message-ID: <907020160.11403426.1525172946040.JavaMail.zimbra@inria.fr>
In-Reply-To: <50c60ddfeb9a44a99f556be2c2ca9a34@BPMBX2013-01.univ-lyon1.fr>

"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.

My understanding is that the succession of events went stg like:

1) invent the rename detection, but consider it experimental
   hence don't activate it by default;

2) add commands using the rename detection, and since it works
   well, use it by default;

3) activate rename detection by default for diff.

The next logical step is what you patch does indeed.

Show 12 quoted lines

> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -161,9 +161,9 @@ static void determine_whence(struct wt_status *s)
> static void status_init_config(struct wt_status *s, config_fn_t fn)
> {
> 	wt_status_prepare(s);
> +	init_diff_ui_defaults();
> 	git_config(fn, s);
> 	determine_whence(s);
> -	init_diff_ui_defaults();
> 	s->hints = advice_status_hints; /* must come after git_config() */
> }

That init_diff_ui_defaults() should indeed have been before git_config() from the beginning. My bad, I'm the one who misplaced it apparently :-(.

Show 10 quoted lines

> --- 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;
> -	rev.diffopt.break_opt = 0;

This "break_opt = 0" deserves a mention in the commit message IMHO. I'm not 100% sure it's a good change actually.

break_opt is normally controlled by "-B/--break-rewrites". I'm not sure why it was set to 0.

-- 
Matthieu Moy
https://matthieu-moy.fr/