Re: [PATCH 2/2] Have the diff-* builtins configure diff before initializing revisions.
From: Jeff King <hidden>
Date: 2017-04-28 08:06:40
On Thu, Apr 27, 2017 at 04:50:37PM -0400, Marc Branchaud wrote:
Subject: [PATCH 2/2] Have the diff-* builtins configure diff before initializing revisions. This makes the commands respect diff configuration options, such as indentHeuristic. Signed-off-by: Marc Branchaud <redacted>
I think it would be helpful to future readers to explain what is going on here. I.e., the bit about calling diff_setup_done(), which copies the globals into the diff struct. The same comments about the subject line from the last patch apply here, too.
builtin/diff-files.c | 2 +- builtin/diff-index.c | 2 +- builtin/diff-tree.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
It would be nice to have a test. Testing that dirstat's permille option has an effect might be the easiest way to do so.
quoted hunk ↗ jump to hunk
diff --git a/builtin/diff-files.c b/builtin/diff-files.c index 15c61fd8d..a572da9d5 100644 --- a/builtin/diff-files.c +++ b/builtin/diff-files.c@@ -20,9 +20,9 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix) int result; unsigned options = 0; + git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ init_revisions(&rev, prefix); gitmodules_config(); - git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ rev.abbrev = 0; precompose_argv(argc, argv);
It's somewhat odd to me that diff_files uses a rev_info struct at all. It doesn't traverse at all, and doesn't respect most of the options. There's a half-hearted attempt to reject some obviously bogus cases, but most of the options are just silently ignored. I think it's mostly a historical wart (especially around the fact that some diff options like combine_merges are in rev_info, which they probably should not be). Anyway, none of that is your problem, and is way outside the scope of this patch. -Peff