[PATCH v3] wt-status: use settings from git_diff_ui_config
If you do something like
- git add .
- git status
- git commit
- git show (or git diff HEAD)
one would expect to have analogous output from git status and git show
(or similar diff-related programs). This is generally not the case, as
git status has hard coded values for diff related options.
With this commit the hard coded settings are dropped from the status
command in favour for values provided by git_diff_ui_config.
What follows are some remarks on the concrete options which were hard
coded in git status:
diffopt.detect_rename
Since the very beginning of git status in a3e870f2e2 ("Add "commit"
helper script", 2005-05-30), git status always used rename detection,
whereas with commands like show and log one had to activate it with a
command line option. After 5404c116aa ("diff: activate diff.renames by
default", 2016-02-25) the default behaves the same by coincidence, but
changing diff.renames to other values can break the consistency between
git status and other commands again. With this commit one control the
same default behaviour with diff.renames.
diffopt.rename_limit
Similarly one has the option diff.renamelimit to adjust this limit for
all commands but git status. With this commit git status will also honor
those.
diffopt.break_opt
Unlike the other two options this cannot be configured by a
configuration option yet. This commit will also change the default
behaviour to not use break rewrites. But as rename detection is most
likely on, this is dangerous to be activated anyway as one can see
here:
https://public-inbox.org/git/xmqqegqaahnh@gitster.dls.corp.google.com/
Signed-off-by: Eckhard S. Maaß <eckhard.s.ma...@gmail.com>
Reviewed-by: Elijah Newren <new...@gmail.com>
---
Hello,
Hopefully I have addressed all issues that have come up so far.
Greetings,
Eckhard
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(_data, >pathspec);
run_diff_index(, 1);
}
@@ -985,7 +982,6 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
setup_revisions(0, NULL, , );
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
- rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
rev.diffopt.file = s->fp;
rev.diffopt.close_file = 0;
/*
--
2.17.0.252.gfe0a9eaf31
[PATCH v2] wt-status: use rename settings from init_diff_ui_defaults
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. 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.
Signed-off-by: Eckhard S. Maaß <eckhard.s.ma...@gmail.com>
Reviewed-by: Elijah Newren <new...@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(_data, >pathspec);
run_diff_index(, 1);
}
@@ -985,7 +982,6 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
setup_revisions(0, NULL, , );
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
- rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
rev.diffopt.file = s->fp;
rev.diffopt.close_file = 0;
/*
--
2.17.0.252.gfe0a9eaf31
[PATCH 1/1] wt-status: use rename settings from init_diff_ui_defaults
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. 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 setting.
With this commit the hard coded rename settings are dropped from the
status command.
Signed-off-by: Eckhard S. Maaß <eckhard.s.ma...@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(_data, >pathspec);
run_diff_index(, 1);
}
@@ -985,7 +982,6 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
setup_revisions(0, NULL, , );
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
- rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
rev.diffopt.file = s->fp;
rev.diffopt.close_file = 0;
/*
--
2.17.0.252.gfe0a9eaf31
[PATCH 0/1] Use the diff.rename configuration for git status
Hello, I have been irritated that the output of git status does not adhere to the same diff settings than the other commands like git show. I think it is more of an oversight and git status is also intended to show the same kind of rename detection (without any more options passed a long the command line) than other similar commands do. This patch should fix that issue. I am aware that this change would also change how the porcelain variations of git status behave for rename detection. However, I could not find any hints that the rename detection should be fixed. As the default would not change (50% rename threshold), this still seems reasonable to me. As for documentation, I am unsure what and where to add: for me it seems like git status should already behave that way, but it is a change and so far none else seemed to have brought it up. Greetings, Eckhard Eckhard S. Maaß (1): wt-status: use rename settings from init_diff_ui_defaults builtin/commit.c | 2 +- t/t4001-diff-rename.sh | 12 wt-status.c| 4 3 files changed, 13 insertions(+), 5 deletions(-) -- 2.17.0.252.gfe0a9eaf31
