From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: "Eckhard S. Maaß" <eckhard.s.maass@googlemail.com>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
"Matthieu Moy" <Matthieu.Moy@imag.fr>,
"Jeff King" <peff@peff.net>, "Elijah Newren" <newren@gmail.com>,
"Ben Peart" <peartben@gmail.com>,
"Eckhard S. Maaß" <eckhard.s.maass@gmail.com>
Subject: Re: [PATCH v2] wt-status: use rename settings from init_diff_ui_defaults
Date: Tue, 01 May 2018 13:00:54 +0200 [thread overview]
Message-ID: <87bmdzzlll.fsf@evledraar.gmail.com> (raw)
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.
> 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@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;
> /*
next prev parent reply other threads:[~2018-05-01 11:01 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <c466854f-6087-e7f1-264a-1d2df9fd9b5a@gmail.com>
2018-05-01 9:49 ` [PATCH v2] wt-status: use rename settings from init_diff_ui_defaults Eckhard S. Maaß
2018-05-01 11:00 ` Ævar Arnfjörð Bjarmason [this message]
2018-05-01 11:37 ` Eckhard Maaß
[not found] ` <50c60ddfeb9a44a99f556be2c2ca9a34@BPMBX2013-01.univ-lyon1.fr>
2018-05-01 11:09 ` Matthieu Moy
2018-05-01 11:43 ` Eckhard Maaß
2018-05-01 12:23 ` Matthieu Moy
2018-05-01 15:52 ` Elijah Newren
2018-05-01 23:11 ` Junio C Hamano
2018-05-02 0:08 ` Elijah Newren
2018-05-02 14:20 ` Ben Peart
2018-05-03 5:22 ` Eckhard Maaß
2018-05-04 11:12 ` [PATCH v3] wt-status: use settings from git_diff_ui_config Eckhard S. Maaß
2018-05-04 15:13 ` Elijah Newren
2018-05-01 16:09 ` [PATCH v2] wt-status: use rename settings from init_diff_ui_defaults Eckhard Maaß
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bmdzzlll.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=Matthieu.Moy@imag.fr \
--cc=eckhard.s.maass@gmail.com \
--cc=eckhard.s.maass@googlemail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=newren@gmail.com \
--cc=peartben@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).