Re: [PATCH v5 0/2] blame: make diff algorithm configurable

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Antonin

On 06/11/2025 22:41, Antonin Delpeuch via GitGitGadget wrote:
> Changes since v4:
> 
>   * hide --minimal option
>   * simplify tests to minimize spun processes
>   * remove redundant XDF_NEED_MINIMAL in bit mask

Excellent, the range-diff below looks good. Thanks for working on this

Phillip

> Antonin Delpeuch (2):
>    xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK
>    blame: make diff algorithm configurable
> 
>   Documentation/diff-algorithm-option.adoc |  20 +++
>   Documentation/diff-options.adoc          |  21 +--
>   Documentation/git-blame.adoc             |   2 +
>   builtin/blame.c                          |  52 +++++-
>   diff.c                                   |   2 -
>   merge-ort.c                              |   2 -
>   t/meson.build                            |   1 +
>   t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++
>   xdiff/xdiff.h                            |   2 +-
>   9 files changed, 279 insertions(+), 26 deletions(-)
>   create mode 100644 Documentation/diff-algorithm-option.adoc
>   create mode 100755 t/t8015-blame-diff-algorithm.sh
> 
> 
> base-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v5
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v5
> Pull-Request: https://github.com/git/git/pull/2075
> 
> Range-diff vs v4:
> 
>   1:  e81a5d2bd2 = 1:  e81a5d2bd2 xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK
>   2:  920a6f3acb ! 2:  60015bbada blame: make diff algorithm configurable
>       @@ builtin/blame.c: static int blame_move_callback(const struct option *option, con
>        +		return error(_("option diff-algorithm accepts \"myers\", "
>        +			       "\"minimal\", \"patience\" and \"histogram\""));
>        +
>       -+	*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);
>       ++	*opt &= ~XDF_DIFF_ALGORITHM_MASK;
>        +	*opt |= value;
>        +
>        +	return 0;
>       @@ builtin/blame.c: int cmd_blame(int argc,
>        -		OPT_BIT(0, "minimal", &xdl_opts, N_("spend extra cycles to find better match"), XDF_NEED_MINIMAL),
>        +		OPT_CALLBACK_F(0, "minimal", &xdl_opts, NULL,
>        +			       N_("spend extra cycles to find a better match"),
>       -+			       PARSE_OPT_NOARG, blame_diff_algorithm_minimal),
>       ++			       PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, blame_diff_algorithm_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")),
>         		OPT_CALLBACK_F('C', NULL, &opt, N_("score"), N_("find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback),
>       @@ t/t8015-blame-diff-algorithm.sh (new)
>        +	EOF
>        +
>        +	git -c diff.algorithm=histogram blame file.c > output &&
>       -+	sed -e "s/^[^ ]* (\([^ ]*\) [^)]*)/\1/g" output > without_varying_parts &&
>       -+	sed -e "s/ *$//g" without_varying_parts > actual &&
>       ++	sed -e "s/^[^ ]* (\([^ ]*\) [^)]*)/\1/g" \
>       ++	    -e "s/ *$//g" output > actual &&
>        +	test_cmp expected actual
>        +'
>        +
>       @@ t/t8015-blame-diff-algorithm.sh (new)
>        +	Commit_2 }
>        +	EOF
>        +
>       -+	git -c diff.algorithm=myers blame file.c --diff-algorithm histogram &&
>       -+	sed -e "s/^[^ ]* (\([^ ]*\) [^)]*)/\1/g" output > without_varying_parts &&
>       -+	sed -e "s/ *$//g" without_varying_parts > actual &&
>       ++	git -c diff.algorithm=myers blame file.c --diff-algorithm histogram > output &&
>       ++	sed -e "s/^[^ ]* (\([^ ]*\) [^)]*)/\1/g" \
>       ++	    -e "s/ *$//g" output > actual &&
>        +	test_cmp expected actual
>        +'
>        +
>