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.
> 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...".
> Signed-off-by: Eckhard S. Maaß <eckhard.s.maass@xxxxxxxxx>
> Reviewed-by: Elijah Newren <newren@xxxxxxxxx>
> ---
> 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;
> /*
