git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 8/8] blame: honor the diff heuristic options and config

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Sep 5, 2016, 09:44 UTC
Message-ID
<8c78cd7079c925f93488850154fc8352db4e4cb8.1473068229.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1473068229.git.mhagger@alum.mit.edu>

Teach "git blame" and "git annotate" the --compaction-heuristic and --indent-heuristic options that are now supported by "git diff".

Also teach them to honor the `diff.compactionHeuristic` and `diff.indentHeuristic` configuration options.

It would be conceivable to introduce separate configuration options for "blame" and "annotate"; for example `blame.compactionHeuristic` and `blame.indentHeuristic`. But it would be confusing to users if blame output is inconsistent with diff output, so it makes more sense for them to respect the same configuration.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 Documentation/diff-heuristic-options.txt |  7 +++++++
 Documentation/diff-options.txt           |  8 +-------
 Documentation/git-annotate.txt           |  1 +
 Documentation/git-blame.txt              |  2 ++
 builtin/blame.c                          | 12 ++++++++++++
 diff.c                                   | 29 +++++++++++++++++------------
 diff.h                                   |  1 +
 t/t4059-diff-indent.sh                   | 29 +++++++++++++++++++++++++++++
 8 files changed, 70 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/diff-heuristic-options.txt
diff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt
new file mode 100644
index 0000000..36cb549
--- /dev/null
+++ b/Documentation/diff-heuristic-options.txt
@@ -0,0 +1,7 @@
+--indent-heuristic::
+--no-indent-heuristic::
+--compaction-heuristic::
+--no-compaction-heuristic::
+	These are to help debugging and tuning experimental heuristics
+	(which are off by default) that shift diff hunk boundaries to
+	make patches easier to read.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index ffc2b60..d6d41e4 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -63,13 +63,7 @@ ifndef::git-format-patch[]
 	Synonym for `-p --raw`.
 endif::git-format-patch[]
 
---indent-heuristic::
---no-indent-heuristic::
---compaction-heuristic::
---no-compaction-heuristic::
-	These are to help debugging and tuning experimental heuristics
-	(which are off by default) that shift diff hunk boundaries to
-	make patches easier to read.
+include::diff-heuristic-options.txt[]
 
 --minimal::
 	Spend extra time to make sure the smallest possible
diff --git a/Documentation/git-annotate.txt b/Documentation/git-annotate.txt
index 05fd482..94be4b8 100644
--- a/Documentation/git-annotate.txt
+++ b/Documentation/git-annotate.txt
@@ -23,6 +23,7 @@ familiar command name for people coming from other SCM systems.
 OPTIONS
 -------
 include::blame-options.txt[]
+include::diff-heuristic-options.txt[]
 
 SEE ALSO
 --------
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ba54175..9dccb33 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -89,6 +89,8 @@ include::blame-options.txt[]
 	abbreviated object name, use <n>+1 digits. Note that 1 column
 	is used for a caret to mark the boundary commit.
 
+include::diff-heuristic-options.txt[]
+
 
 THE PORCELAIN FORMAT
 --------------------
diff --git a/builtin/blame.c b/builtin/blame.c
index 7ec7823..f7b67f8 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2220,6 +2220,8 @@ static int git_blame_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (git_diff_heuristic_config(var, value, cb) < 0)
+		return -1;
 	if (userdiff_config(var, value) < 0)
 		return -1;
 
@@ -2549,6 +2551,15 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		OPT_BIT('s', NULL, &output_option, N_("Suppress author name and timestamp (Default: off)"), OUTPUT_NO_AUTHOR),
 		OPT_BIT('e', "show-email", &output_option, N_("Show author email instead of name (Default: off)"), OUTPUT_SHOW_EMAIL),
 		OPT_BIT('w', NULL, &xdl_opts, N_("Ignore whitespace differences"), XDF_IGNORE_WHITESPACE),
+
+		/*
+		 * The following two options are parsed by parse_revision_opt()
+		 * and are only included here to get included in the "-h"
+		 * output:
+		 */
+		{ OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental indent-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
+		{ OPTION_LOWLEVEL_CALLBACK, 0, "compaction-heuristic", NULL, NULL, N_("Use an experimental blank-line-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
+
 		OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
 		OPT_STRING('S', NULL, &revs_file, N_("file"), N_("Use revisions from <file> instead of calling git-rev-list")),
 		OPT_STRING(0, "contents", &contents_from, N_("file"), N_("Use <file>'s contents as the final image")),
@@ -2595,6 +2606,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	}
 parse_done:
 	no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
+	xdl_opts |= revs.diffopt.xdl_opts & (XDF_COMPACTION_HEURISTIC | XDF_INDENT_HEURISTIC);
 	DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
 	argc = parse_options_end(&ctx);
 
diff --git a/diff.c b/diff.c
index 8b15ed7..8a2003d 100644
--- a/diff.c
+++ b/diff.c
@@ -175,6 +175,21 @@ void init_diff_ui_defaults(void)
 	diff_detect_rename_default = 1;
 }
 
+int git_diff_heuristic_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "diff.indentheuristic")) {
+		diff_indent_heuristic = git_config_bool(var, value);
+		if (diff_indent_heuristic)
+			diff_compaction_heuristic = 0;
+	}
+	if (!strcmp(var, "diff.compactionheuristic")) {
+		diff_compaction_heuristic = git_config_bool(var, value);
+		if (diff_compaction_heuristic)
+			diff_indent_heuristic = 0;
+	}
+	return 0;
+}
+
 int git_diff_ui_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
@@ -191,18 +206,6 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_detect_rename_default = git_config_rename(var, value);
 		return 0;
 	}
-	if (!strcmp(var, "diff.indentheuristic")) {
-		diff_indent_heuristic = git_config_bool(var, value);
-		if (diff_indent_heuristic)
-			diff_compaction_heuristic = 0;
-		return 0;
-	}
-	if (!strcmp(var, "diff.compactionheuristic")) {
-		diff_compaction_heuristic = git_config_bool(var, value);
-		if (diff_compaction_heuristic)
-			diff_indent_heuristic = 0;
-		return 0;
-	}
 	if (!strcmp(var, "diff.autorefreshindex")) {
 		diff_auto_refresh_index = git_config_bool(var, value);
 		return 0;
@@ -243,6 +246,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (git_diff_heuristic_config(var, value, cb) < 0)
+		return -1;
 	if (git_color_config(var, value, cb) < 0)
 		return -1;
 
diff --git a/diff.h b/diff.h
index 7883729..1ecc9b6 100644
--- a/diff.h
+++ b/diff.h
@@ -266,6 +266,7 @@ extern int parse_long_opt(const char *opt, const char **argv,
 			 const char **optarg);
 
 extern int git_diff_basic_config(const char *var, const char *value, void *cb);
+extern int git_diff_heuristic_config(const char *var, const char *value, void *cb);
 extern void init_diff_ui_defaults(void);
 extern int git_diff_ui_config(const char *var, const char *value, void *cb);
 extern void diff_setup(struct diff_options *);
diff --git a/t/t4059-diff-indent.sh b/t/t4059-diff-indent.sh
index 0a65b58..5564506 100755
--- a/t/t4059-diff-indent.sh
+++ b/t/t4059-diff-indent.sh
@@ -14,6 +14,14 @@ compare_diff () {
 	test_cmp .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
 }
 
+# Compare blame output using the expectation for a diff as reference.
+# Only look for the lines coming from non-boundary commits.
+compare_blame () {
+	sed -n -e "1,4d" -e "s/^\+//p" <"$1" >.tmp-1
+	sed -ne "s/^[^^][^)]*) *//p" <"$2" >.tmp-2
+	test_cmp .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
+}
+
 test_expect_success 'prepare' '
 	cat <<-\EOF >spaces.txt &&
 	1
@@ -184,4 +192,25 @@ test_expect_success 'diff: nice functions with --indent-heuristic' '
 	compare_diff functions-compacted-expect out-compacted
 '
 
+test_expect_success 'blame: ugly spaces' '
+	git blame old..new -- spaces.txt >out-blame &&
+	compare_blame spaces-expect out-blame
+'
+
+test_expect_success 'blame: nice spaces with --indent-heuristic' '
+	git blame --indent-heuristic old..new -- spaces.txt >out-blame-compacted &&
+	compare_blame spaces-compacted-expect out-blame-compacted
+'
+
+test_expect_success 'blame: nice spaces with diff.indentHeuristic' '
+	git -c diff.indentHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&
+	compare_blame spaces-compacted-expect out-blame-compacted2
+'
+
+test_expect_success 'blame: --no-indent-heuristic overrides config' '
+	git -c diff.indentHeuristic=true blame --no-indent-heuristic old..new -- spaces.txt >out-blame2 &&
+	git blame old..new -- spaces.txt >out-blame &&
+	compare_blame spaces-expect out-blame2
+'
+
 test_done

-- 
2.9.3
Previous: Michael HaggertyNext: Michael Haggerty
Message 7 of 16 in “Better heuristics make prettier diffs”
  1. 0/8 Better heuristics make prettier diffsMichael Haggerty, Sep 5, 2016
  2. 1/8 xdl_change_compact(): fix compaction heuristic to adjust ixoMichael Haggerty, Sep 5, 2016
  3. 7/8 parse-options: add parse_opt_unknown_cb()Michael Haggerty, Sep 5, 2016
  4. 3/8 is_blank_line(): take a single xrecord_t as argumentMichael Haggerty, Sep 5, 2016
  5. 2/8 xdl_change_compact(): only use heuristic if group can't be matchedMichael Haggerty, Sep 5, 2016
  6. 5/8 xdl_change_compact(): introduce the concept of a change groupMichael Haggerty, Sep 5, 2016
  7. 8/8 blame: honor the diff heuristic options and configMichael Haggerty, Sep 5, 2016
  8. 6/8 diff: improve positioning of add/delete blocks in diffsMichael Haggerty, Sep 5, 2016
  9. 4/8 recs_match(): take two xrecord_t pointers as argumentsMichael Haggerty, Sep 5, 2016
  10. Junio C HamanoSep 7, 2016
  11. Jacob KellerSep 7, 2016
  12. Junio C HamanoSep 7, 2016
  13. Michael HaggertySep 19, 2016
  14. Junio C HamanoSep 19, 2016
  15. Junio C HamanoSep 19, 2016
  16. Michael HaggertySep 20, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.