git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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;
>  	/*

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