Re: [PATCH v2] wt-status: use rename settings from init_diff_ui_defaults
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- May 1, 2018, 11:00 UTC
- Message-ID
- <87bmdzzlll.fsf@evledraar.gmail.com>
- In-Reply-To
- <20180501094940.17772-1-eckhard.s.maass@gmail.com>
On Tue, May 01 2018, Eckhard S. Maaß 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.
Can you elaborate on this? It seems initial rename detection was added in 5c97558c9a ("[PATCH] Detect renames in diff family.", 2005-05-19) and the first version of the status script added by Linus in a3e870f2e2 ("Add "commit" helper script", 2005-05-30), and that one piggy-backs on "diff" for rename detection.
So didn't we use diff heuristics to begin with, and then regressed? I've only given this a skimming, but it's useful to have that sort of historical context mentioned explicitly with commit ids.
Show 14 quoted lines
> 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 and diff.renameLimit setting.
>
> With this commit the hard coded settings are dropped from the status
> command.It's unclear to me what this means, so the only difference between "status" and "diff" is that the former had a hardcoded limit of 200? In that case it was added at 100 (later adusted) in 0024a54923 ("Fix the rename detection limit checking", 2007-09-14), so not since "the very beginning...".
Show 68 quoted lines
> Signed-off-by: Eckhard S. Maaß <eckhard.s.maass@gmail.com>
> Reviewed-by: Elijah Newren <newren@gmail.com>
> ---
> builtin/commit.c | 2 +-
> t/t4001-diff-rename.sh | 12 ++++++++++++
> wt-status.c | 4 ----
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 5571d4a3e2..5240f11225 100644
> --- 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() */
> }
>
> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
> index a07816d560..bf4030371a 100755
> --- a/t/t4001-diff-rename.sh
> +++ b/t/t4001-diff-rename.sh
> @@ -138,6 +138,18 @@ test_expect_success 'favour same basenames over different ones' '
> test_i18ngrep "renamed: .*path1 -> subdir/path1" out
> '
>
> +test_expect_success 'test diff.renames=true for git status' '
> + git -c diff.renames=true status >out &&
> + test_i18ngrep "renamed: .*path1 -> subdir/path1" out
> +'
> +
> +test_expect_success 'test diff.renames=false for git status' '
> + git -c diff.renames=false status >out &&
> + test_i18ngrep ! "renamed: .*path1 -> subdir/path1" out &&
> + test_i18ngrep "new file: .*subdir/path1" out &&
> + test_i18ngrep "deleted: .*[^/]path1" out
> +'
> +
> test_expect_success 'favour same basenames even with minor differences' '
> git show HEAD:path1 | sed "s/15/16/" > subdir/path1 &&
> git status >out &&
> 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;
> - rev.diffopt.break_opt = 0;
> copy_pathspec(&rev.prune_data, &s->pathspec);
> run_diff_index(&rev, 1);
> }
> @@ -985,7 +982,6 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
> setup_revisions(0, NULL, &rev, &opt);
>
> rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
> - rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
> rev.diffopt.file = s->fp;
> rev.diffopt.close_file = 0;
> /*