{"thread":{"id":"48383","subject":"[PATCH 0/1] Use the diff.rename configuration for git status","startedAt":"2018-04-30T09:34:42Z","lastAt":"2018-04-30T20:06:10Z","messageCount":3,"participants":["Eckhard S. Maaß","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"346130","messageId":"20180430093421.27551-1-eckhard.s.maass@gmail.com","threadId":"48383","inReplyTo":null,"subject":"[PATCH 0/1] Use the diff.rename configuration for git status","fromName":"Eckhard S. Maaß","fromEmail":"eckhard.s.maass@googlemail.com","sentAt":"2018-04-30T09:34:20Z","receivedAt":"2018-04-30T09:34:42Z","isPatch":true,"sender":{"key":"eckhard.s.maass@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/21984134?v=4"},"body":"Hello,\n\nI have been irritated that the output of git status does not adhere to\nthe same diff settings than the other commands like git show. I think it\nis more of an oversight and git status is also intended to show the same\nkind of rename detection (without any more options passed a long the\ncommand line) than other similar commands do. This patch should fix that\nissue.\n\nI am aware that this change would also change how the porcelain\nvariations of git status behave for rename detection. However, I could\nnot find any hints that the rename detection should be fixed. As the\ndefault would not change (50% rename threshold), this still seems\nreasonable to me.\n\nAs for documentation, I am unsure what and where to add: for me it seems\nlike git status should already behave that way, but it is a change and\nso far none else seemed to have brought it up.\n\nGreetings,\nEckhard\n\nEckhard S. Maaß (1):\n  wt-status: use rename settings from init_diff_ui_defaults\n\n builtin/commit.c       |  2 +-\n t/t4001-diff-rename.sh | 12 ++++++++++++\n wt-status.c            |  4 ----\n 3 files changed, 13 insertions(+), 5 deletions(-)\n\n-- \n2.17.0.252.gfe0a9eaf31\n\n"},{"id":"346131","messageId":"20180430093421.27551-2-eckhard.s.maass@gmail.com","threadId":"48383","inReplyTo":"20180430093421.27551-1-eckhard.s.maass@gmail.com","subject":"[PATCH 1/1] wt-status: use rename settings from init_diff_ui_defaults","fromName":"Eckhard S. Maaß","fromEmail":"eckhard.s.maass@googlemail.com","sentAt":"2018-04-30T09:34:21Z","receivedAt":"2018-04-30T09:34:46Z","isPatch":true,"sender":{"key":"eckhard.s.maass@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/21984134?v=4"},"body":"Since the very beginning, git status behaved differently for rename\ndetection than other rename aware commands like git log or git show as\nit has the use of rename hard coded into it.  After 5404c116aa (\"diff:\nactivate diff.renames by default\", 2016-02-25) the default behaves the\nsame by coincidence, but a work flow like\n\n    - git add .\n    - git status\n    - git commit\n    - git show\n\nshould give you the same information on renames (and/or copies if\nactivated) accordingly to the diff.renames setting.\n\nWith this commit the hard coded rename settings are dropped from the\nstatus command.\n\nSigned-off-by: Eckhard S. Maaß <eckhard.s.maass@gmail.com>\n---\n builtin/commit.c       |  2 +-\n t/t4001-diff-rename.sh | 12 ++++++++++++\n wt-status.c            |  4 ----\n 3 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 5571d4a3e2..5240f11225 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -161,9 +161,9 @@ static void determine_whence(struct wt_status *s)\n static void status_init_config(struct wt_status *s, config_fn_t fn)\n {\n \twt_status_prepare(s);\n+\tinit_diff_ui_defaults();\n \tgit_config(fn, s);\n \tdetermine_whence(s);\n-\tinit_diff_ui_defaults();\n \ts->hints = advice_status_hints; /* must come after git_config() */\n }\n \ndiff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\nindex a07816d560..bf4030371a 100755\n--- a/t/t4001-diff-rename.sh\n+++ b/t/t4001-diff-rename.sh\n@@ -138,6 +138,18 @@ test_expect_success 'favour same basenames over different ones' '\n \ttest_i18ngrep \"renamed: .*path1 -> subdir/path1\" out\n '\n \n+test_expect_success 'test diff.renames=true for git status' '\n+\tgit -c diff.renames=true status >out &&\n+\ttest_i18ngrep \"renamed: .*path1 -> subdir/path1\" out\n+'\n+\n+test_expect_success 'test diff.renames=false for git status' '\n+\tgit -c diff.renames=false status >out &&\n+\ttest_i18ngrep ! \"renamed: .*path1 -> subdir/path1\" out &&\n+\ttest_i18ngrep \"new file: .*subdir/path1\" out &&\n+\ttest_i18ngrep \"deleted: .*[^/]path1\" out\n+'\n+\n test_expect_success 'favour same basenames even with minor differences' '\n \tgit show HEAD:path1 | sed \"s/15/16/\" > subdir/path1 &&\n \tgit status >out &&\ndiff --git a/wt-status.c b/wt-status.c\nindex 50815e5faf..32f3bcaebd 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -625,9 +625,6 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = wt_status_collect_updated_cb;\n \trev.diffopt.format_callback_data = s;\n-\trev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n-\trev.diffopt.rename_limit = 200;\n-\trev.diffopt.break_opt = 0;\n \tcopy_pathspec(&rev.prune_data, &s->pathspec);\n \trun_diff_index(&rev, 1);\n }\n@@ -985,7 +982,6 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, &opt);\n \n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n-\trev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n \trev.diffopt.file = s->fp;\n \trev.diffopt.close_file = 0;\n \t/*\n-- \n2.17.0.252.gfe0a9eaf31\n\n"},{"id":"346190","messageId":"CABPp-BFbV0rmDjxJL4m7v+dJ=kFV7g+6DCgPbSMn0Oqx-3PPmA@mail.gmail.com","threadId":"48383","inReplyTo":"20180430093421.27551-2-eckhard.s.maass@gmail.com","subject":"Re: [PATCH 1/1] wt-status: use rename settings from init_diff_ui_defaults","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-04-30T19:27:23Z","receivedAt":"2018-04-30T20:06:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Eckhard,\n\nOn Mon, Apr 30, 2018 at 2:34 AM, Eckhard S. Maaß\n<eckhard.s.maass@googlemail.com> wrote:\n> Since the very beginning, git status behaved differently for rename\n> detection than other rename aware commands like git log or git show as\n> it has the use of rename hard coded into it.  After 5404c116aa (\"diff:\n> activate diff.renames by default\", 2016-02-25) the default behaves the\n> same by coincidence, but a work flow like\n>\n>     - git add .\n>     - git status\n>     - git commit\n>     - git show\n>\n> should give you the same information on renames (and/or copies if\n> activated) accordingly to the diff.renames setting.\n\nThanks for sending this change in.  I agree with the logic.  I think\nthe last sentence needs a s/diff.renames setting/diff.renames and\ndiff.renameLimit settings/, though, because...\n\n> With this commit the hard coded rename settings are dropped from the\n> status command.\n>\n> Signed-off-by: Eckhard S. Maaß <eckhard.s.maass@gmail.com>\n> ---\n>  builtin/commit.c       |  2 +-\n>  t/t4001-diff-rename.sh | 12 ++++++++++++\n>  wt-status.c            |  4 ----\n>  3 files changed, 13 insertions(+), 5 deletions(-)\n>\n<snip>\n> diff --git a/wt-status.c b/wt-status.c\n> index 50815e5faf..32f3bcaebd 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -625,9 +625,6 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n>         rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n>         rev.diffopt.format_callback = wt_status_collect_updated_cb;\n>         rev.diffopt.format_callback_data = s;\n> -       rev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n> -       rev.diffopt.rename_limit = 200;\n\nBy removing the hard-coded value of 200, the rename limit will instead\nmatch whatever the user specified (or, if they didn't specify\nanything, then the value of 400 from diff_rename_limit_default).  I\nthink that's a good change, for the exact same reasons as you argue\nfor making diff.renames be used everywhere in the commit message, it\njust needs to be mentioned in the commit message.\n\nEverything else in the patch looks good to me, so with that change\nfeel free to add:\n\nReviewed-by: Elijah Newren <newren@gmail.com>\n"}]}