{"thread":{"id":"64356","subject":"[PATCH] blame: make diff algorithm configurable","startedAt":"2025-10-20T14:56:05Z","lastAt":"2025-11-17T18:24:07Z","messageCount":32,"participants":["Antonin Delpeuch via GitGitGadget","Junio C Hamano","Antonin Delpeuch","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"529170","messageId":"pull.2075.git.git.1760972162827.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":null,"subject":"[PATCH] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-10-20T14:56:02Z","receivedAt":"2025-10-20T14:56:05Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe diff algorithm used in 'git-blame(1)' can be configured using the\n`--diff-algorithm` option or the `diff.algorithm` config variable.\nMyers diff remains the default.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n    blame: make diff algorithm configurable\n    \n    There has been long-standing interest in changing the default diff\n    algorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\n    for that: https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n    \n    As a preparation, it is worth making sure that the diff algorithm is\n    configurable where useful. It can have significant impact on the output\n    of the git-blame command, so I propose to make it configurable there\n    too. I have followed the convention of other commands (such as git-diff)\n    to introduce a --diff-algorithm option.\n    \n    I understand that this command is a user-facing (porcelain) one, so I\n    think making it honor the diff.algorithm UI config variable is also\n    appropriate. The git-blame command has a machine-readable format that\n    can be enabled with --porcelain (which should be called --plumbing if\n    you ask me) so I wonder if the diff.algorithm variable should still be\n    honored in this case, as there could be the desire to keep it\n    independent from UI config variables (similarly to git-merge-file, a\n    plumbing command which doesn't honor diff.algorithm).\n    \n    If the general idea of this patch is judged worthwhile, I would be happy\n    to add tests to demonstrate the impact of the diff algorithm on blame\n    output.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v1\nPull-Request: https://github.com/git/git/pull/2075\n\n Documentation/git-blame.adoc | 21 ++++++++++++++++++++\n builtin/blame.c              | 38 +++++++++++++++++++++++++++++++++++-\n 2 files changed, 58 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex e438d28625..4beb2df551 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -85,6 +85,27 @@ include::blame-options.adoc[]\n \tIgnore whitespace when comparing the parent's version and\n \tthe child's to find where the lines came from.\n \n+`--diff-algorithm=(patience|minimal|histogram|myers)`::\n+\tChoose a diff algorithm. The variants are as follows:\n++\n+--\n+   `default`;;\n+   `myers`;;\n+\tThe basic greedy diff algorithm. Currently, this is the default.\n+   `minimal`;;\n+\tSpend extra time to make sure the smallest possible diff is\n+\tproduced.\n+   `patience`;;\n+\tUse \"patience diff\" algorithm when generating patches.\n+   `histogram`;;\n+\tThis algorithm extends the patience algorithm to \"support\n+\tlow-occurrence common elements\".\n+--\n++\n+For instance, if you configured the `diff.algorithm` variable to a\n+non-default value and want to use the default one, then you\n+have to use `--diff-algorithm=default` option.\n+\n --abbrev=<n>::\n \tInstead of using the default 7+1 hexadecimal digits as the\n \tabbreviated object name, use <m>+1 digits, where <m> is at\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 2703820258..177b606e81 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n \t\t}\n \t}\n \n+\tif (!strcmp(var, \"diff.algorithm\")) {\n+\t\tlong diff_algorithm;\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tdiff_algorithm = parse_algorithm_value(value);\n+\t\tif (diff_algorithm < 0)\n+\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n+\t\t\t\t     var, value);\n+\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t\txdl_opts |= diff_algorithm;\n+\t\treturn 0;\n+\t}\n+\n \tif (git_diff_heuristic_config(var, value, cb) < 0)\n \t\treturn -1;\n \tif (userdiff_config(var, value) < 0)\n@@ -824,6 +837,26 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n \treturn 0;\n }\n \n+static int blame_diff_algorithm_callback(const struct option *option,\n+\t\t\t\t\t const char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\tlong value = parse_algorithm_value(arg);\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tif (value < 0)\n+\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n+\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n+\n+\t// ignore any previous --minimal setting, following git-diff's behavior\n+\t*opt &= ~XDF_NEED_MINIMAL;\n+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t*opt |= value;\n+\n+\treturn 0;\n+}\n+\n static int is_a_rev(const char *name)\n {\n \tstruct object_id oid;\n@@ -908,13 +941,16 @@ int cmd_blame(int argc,\n \t\tOPT_BIT('f', \"show-name\", &output_option, N_(\"show original filename (Default: auto)\"), OUTPUT_SHOW_NAME),\n \t\tOPT_BIT('n', \"show-number\", &output_option, N_(\"show original linenumber (Default: off)\"), OUTPUT_SHOW_NUMBER),\n \t\tOPT_BIT('p', \"porcelain\", &output_option, N_(\"show in a format designed for machine consumption\"), OUTPUT_PORCELAIN),\n-\t\tOPT_BIT(0, \"line-porcelain\", &output_option, N_(\"show porcelain format with per-line commit information\"), OUTPUT_PORCELAIN|OUTPUT_LINE_PORCELAIN),\n+\t\tOPT_BIT(0, \"line-porcelain\", &output_option, N_(\"show porcelain format with per-line commit information\"), OUTPUT_PORCELAIN | OUTPUT_LINE_PORCELAIN),\n \t\tOPT_BIT('c', NULL, &output_option, N_(\"use the same output mode as git-annotate (Default: off)\"), OUTPUT_ANNOTATE_COMPAT),\n \t\tOPT_BIT('t', NULL, &output_option, N_(\"show raw timestamp (Default: off)\"), OUTPUT_RAW_TIMESTAMP),\n \t\tOPT_BIT('l', NULL, &output_option, N_(\"show long commit SHA1 (Default: off)\"), OUTPUT_LONG_OBJECT_NAME),\n \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n+\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n+\t\t\t       N_(\"choose a diff algorithm\"),\n+\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n\nbase-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\n-- \ngitgitgadget\n"},{"id":"529174","messageId":"xmqqldl51rtm.fsf@gitster.g","threadId":"64356","inReplyTo":"pull.2075.git.git.1760972162827.gitgitgadget@gmail.com","subject":"Re: [PATCH] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-20T16:05:57Z","receivedAt":"2025-10-20T16:06:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Antonin Delpeuch via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Antonin Delpeuch <antonin@delpeuch.eu>\n>\n> The diff algorithm used in 'git-blame(1)' can be configured using the\n> `--diff-algorithm` option or the `diff.algorithm` config variable.\n> Myers diff remains the default.\n\nThe usual way to compose a log message of this project is to\n\n - Give an observation on how the current system works in the\n   present tense (so no need to say \"Currently X is Y\", or\n   \"Previously X was Y\" to describe the state before your change;\n   just \"X is Y\" is enough), and discuss what you perceive as a\n   problem in it.\n\n - Propose a solution (optional---often, problem description\n   trivially leads to an obvious solution in reader's minds).\n\n - Give commands to somebody editing the codebase to \"make it so\",\n   instead of saying \"This commit does X\".\n\nin this order.  This hasn't changed since your first commit to this\nproject a few years ago.\n\nAnd when read with that expectation, I was surprised that \"blame\"\nalready paid attention to the command line option and configuration\nvariable, as that paragraph was supposed to explain what happens\nwithout the patch being proposed.  It was a pleasant surprise that\nturned out to be untrue X-<.\n\n> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n> ---\n>     blame: make diff algorithm configurable\n>     \n>     There has been long-standing interest in changing the default diff\n>     algorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\n>     for that: https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n>     \n>     As a preparation, it is worth making sure that the diff algorithm is\n>     configurable where useful. It can have significant impact on the output\n>     of the git-blame command, so I propose to make it configurable there\n>     too. I have followed the convention of other commands (such as git-diff)\n>     to introduce a --diff-algorithm option.\n\nAll of the above are good materials to be in the proposed log\nmessage, not under the three-dash line, to explain the motivation\nbehind the change.\n\n>     I understand that this command is a user-facing (porcelain) one, so I\n>     think making it honor the diff.algorithm UI config variable is also\n>     appropriate. The git-blame command has a machine-readable format that\n>     can be enabled with --porcelain (which should be called --plumbing if\n>     you ask me) so I wonder if the diff.algorithm variable should still be\n>     honored in this case, as there could be the desire to keep it\n>     independent from UI config variables (similarly to git-merge-file, a\n>     plumbing command which doesn't honor diff.algorithm).\n\nGood consideration and something we should make sure we do the right\nthing for our users.  Personally, I would not be concerned---the\nonly folks that possibly affected are those who save old blame\noutput and wants a fresh \"git blame\" run they make today would\nproduce bit-for-bit identical output, but if they use newer versions\nof Git with improved xdiff implementation, they cannot expect that\nwith or without the configuration knob _anyway_.  This is just my\npersonal opinion.  Others may differ.\n\n>     If the general idea of this patch is judged worthwhile, I would be happy\n>     to add tests to demonstrate the impact of the diff algorithm on blame\n>     output.\n\nDo not ever say this here.\n\nI've seen from time to time people ask \"I am thinking of doing this;\nwill a patch be accepted?  If so, I'll work on it.\" before showing\nany work, and my response always has been:\n\n (1) We don't know how useful and interesting your contribution would\n     be for our audience, until we see it; and\n\n (2) If you truly believe in your work (find it useful, find writing\n     it fun, etc.), that would be incentive enough for you to work\n     on it, whether or not the result will land in my tree.  You\n     should instead aim for something so brilliant that we would\n     come to you begging for your permission to include it in our\n     project.\n\n> diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\n> index e438d28625..4beb2df551 100644\n> --- a/Documentation/git-blame.adoc\n> +++ b/Documentation/git-blame.adoc\n> @@ -85,6 +85,27 @@ include::blame-options.adoc[]\n>  \tIgnore whitespace when comparing the parent's version and\n>  \tthe child's to find where the lines came from.\n>  \n> +`--diff-algorithm=(patience|minimal|histogram|myers)`::\n> +\tChoose a diff algorithm. The variants are as follows:\n> ++\n> +--\n> +   `default`;;\n> +   `myers`;;\n> +\tThe basic greedy diff algorithm. Currently, this is the default.\n> +   `minimal`;;\n> +\tSpend extra time to make sure the smallest possible diff is\n> +\tproduced.\n> +   `patience`;;\n> +\tUse \"patience diff\" algorithm when generating patches.\n> +   `histogram`;;\n> +\tThis algorithm extends the patience algorithm to \"support\n> +\tlow-occurrence common elements\".\n> +--\n> ++\n> +For instance, if you configured the `diff.algorithm` variable to a\n> +non-default value and want to use the default one, then you\n> +have to use `--diff-algorithm=default` option.\n\nIs this copied from somewhere else, or did you come up with the\nabove text yourself?  If the former, perhaps it is a good idea to\nreduce the duplicattion.  Use of \"include::line-range-format.adoc[]\"\nin Documentation/blame-options.adoc (which in turn is included by\nDocumentation/git-blame.adoc) may serve as a good model to include\nthe same text in multiple places (the \"line-range\" syntax thing is\nincluded directly or indirectly and its text appears in a handful of\nplaces as the result).  Copy the original text out into a new file\nto be included (say, \"diff-algorithm-option.adoc\"), replace the\noriginal text with \"include::diff-algorithm-option.adoc[]\", and then\nadd another \"include::diff-algorithm-option.adoc[]\" here in\ngit-blame documentation instead of duplicating the text like the\nabove hunk does.\n\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 2703820258..177b606e81 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n>  \t\t}\n>  \t}\n>  \n> +\tif (!strcmp(var, \"diff.algorithm\")) {\n> +\t\tlong diff_algorithm;\n> +\t\tif (!value)\n> +\t\t\treturn config_error_nonbool(var);\n> +\t\tdiff_algorithm = parse_algorithm_value(value);\n> +\t\tif (diff_algorithm < 0)\n> +\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n> +\t\t\t\t     var, value);\n\nOK, this message is copied from git_diff_ui_config(), which is where\n\"git log\" and 4 commands in the \"git diff\" family gets their error\nmessage when \"git -c diff.algorithm=bogus <cmd>\" is run.  It is a\nbit suboptimal, but users would know how to read the documentation\n(even though in practice they never do), so let's say this is OK, at\nleast for now.\n\n    For future reference (note: this is a #leftoverbits comment that is\n    left here for the benefit of those who scan the list archive for\n    ideas on what to do when they are absolutely bored without anything\n    interesting to do, not meant as a suggestion to do anything of this\n    sort before this patch lands), in addition to \"git log\" and 4\n    commands in the \"git diff\" family,\n\n     - merge-ort.c has the same message.\n     - builtin/merge-file.c gives a bit nicer message but that is a bit\n       of maintenance burden.\n     - curiously \"git log\" and four commands in the \"git diff\" family\n       give a much nicer message when a --diff-algorithm=bogus is given\n       from the command line, but not in the configuration file.\n\n    we may want to consolidate the error message into one place (a\n    constant or \"extern const char *diff_algorithm_error_message\",\n    or something else that is i18n friendly) and use it from all these\n    places I just identified.\n\n> +\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n> +\t\txdl_opts |= diff_algorithm;\n> +\t\treturn 0;\n> +\t}\n\n>  \tif (git_diff_heuristic_config(var, value, cb) < 0)\n>  \t\treturn -1;\n\nThis one relies on git_diff_heuristic_config() to give message when\nit returns negative, so we do not have to do anything.  OK.\n\n    Contination of the above #leftoverbits may be to see if\n    parse_algorithm_value() is a good place to consolidate the error\n    message, after auditing all its callers (if such a change turns\n    out to be a good idea, they need to lose their own messages).\n\n> @@ -824,6 +837,26 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n>  \treturn 0;\n>  }\n>  \n> +static int blame_diff_algorithm_callback(const struct option *option,\n> +\t\t\t\t\t const char *arg, int unset)\n> +{\n> +\tint *opt = option->value;\n> +\tlong value = parse_algorithm_value(arg);\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\n> +\tif (value < 0)\n> +\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n> +\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n\nYou inherited the same \"config error gets a message that requires\nusers to consult the manual, option error gets something a bit more\nuseful but is a maintenance burden\" trait from \"git diff\" and family\nhere.  Let's say this is OK, too, at least for now.\n\n> +\t// ignore any previous --minimal setting, following git-diff's behavior\n\nWe do not do // comments around here, outside borrowed code.\n\n> +\t*opt &= ~XDF_NEED_MINIMAL;\n> +\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n> +\t*opt |= value;\n> +\n> +\treturn 0;\n> +}\n> +\n>  static int is_a_rev(const char *name)\n>  {\n>  \tstruct object_id oid;\n> @@ -908,13 +941,16 @@ int cmd_blame(int argc,\n>  \t\tOPT_BIT('f', \"show-name\", &output_option, N_(\"show original filename (Default: auto)\"), OUTPUT_SHOW_NAME),\n>  \t\tOPT_BIT('n', \"show-number\", &output_option, N_(\"show original linenumber (Default: off)\"), OUTPUT_SHOW_NUMBER),\n>  \t\tOPT_BIT('p', \"porcelain\", &output_option, N_(\"show in a format designed for machine consumption\"), OUTPUT_PORCELAIN),\n> -\t\tOPT_BIT(0, \"line-porcelain\", &output_option, N_(\"show porcelain format with per-line commit information\"), OUTPUT_PORCELAIN|OUTPUT_LINE_PORCELAIN),\n> +\t\tOPT_BIT(0, \"line-porcelain\", &output_option, N_(\"show porcelain format with per-line commit information\"), OUTPUT_PORCELAIN | OUTPUT_LINE_PORCELAIN),\n\nWHY?\n\n>  \t\tOPT_BIT('c', NULL, &output_option, N_(\"use the same output mode as git-annotate (Default: off)\"), OUTPUT_ANNOTATE_COMPAT),\n>  \t\tOPT_BIT('t', NULL, &output_option, N_(\"show raw timestamp (Default: off)\"), OUTPUT_RAW_TIMESTAMP),\n>  \t\tOPT_BIT('l', NULL, &output_option, N_(\"show long commit SHA1 (Default: off)\"), OUTPUT_LONG_OBJECT_NAME),\n>  \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n>  \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n>  \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n> +\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n> +\t\t\t       N_(\"choose a diff algorithm\"),\n> +\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n\nOK.\n\n>  \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n>  \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n>  \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n>\n> base-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\n"},{"id":"529406","messageId":"d59a2f97-1a69-44f6-924e-7419e36329a0@delpeuch.eu","threadId":"64356","inReplyTo":"xmqqldl51rtm.fsf@gitster.g","subject":"Re: [PATCH] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch","fromEmail":"antonin@delpeuch.eu","sentAt":"2025-10-22T09:37:46Z","receivedAt":"2025-10-22T09:37:54Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"On 20/10/2025 18:05, Junio C Hamano wrote:\n\n>>      If the general idea of this patch is judged worthwhile, I would be happy\n>>      to add tests to demonstrate the impact of the diff algorithm on blame\n>>      output.\n> Do not ever say this here.\n>\n> I've seen from time to time people ask \"I am thinking of doing this;\n> will a patch be accepted?  If so, I'll work on it.\" before showing\n> any work, and my response always has been:\n>\n>   (1) We don't know how useful and interesting your contribution would\n>       be for our audience, until we see it; and\n>\n>   (2) If you truly believe in your work (find it useful, find writing\n>       it fun, etc.), that would be incentive enough for you to work\n>       on it, whether or not the result will land in my tree.  You\n>       should instead aim for something so brilliant that we would\n>       come to you begging for your permission to include it in our\n>       project.\n\nI am surprised by your reaction here, both by its substance and form.\n\nMy understanding is that gathering feedback on a proposal before \ncarrying out the implementation work in its entirety is widely accepted \nas a good practice for contributions to open source projects. For \ninstance, the following guide encourages to do so (in GitHub terms, by \nproposing an improvement as an issue first, and by opening a draft pull \nrequest if necessary):\n\nhttps://opensource.guide/how-to-contribute/\n\nWhile this is phrased in the context of GitHub, I think the general \nprinciple behind it is healthy. In projects I maintain, I feel bad for \ncontributors who submit contributions that clearly required a \nsignificant effort, but that I can't accept for certain reasons. I wish \nthey had got in touch ahead of investing all this work, because I care \nabout their time.\n\nIn fact, I already did so for an earlier contribution to this very \nproject, and on that occasion you did not seem to take offense at the \nfact that my proposal was done without an accompanying patch:\n\nhttps://lore.kernel.org/git/8bb5e41e-4db9-4527-8492-3aca6a0f40bf@delpeuch.eu/\n\nHas your position changed since? Or did I benefit from more of your \nkindness back then as a new contributor?\n\nYour argument about my work being worthy on its own even if it's not \nintegrated to your tree is an interesting one, but let me expand on my \nmotivation for this patch. This change is not something I personally \nneed, nor something that is particularly fun to write. I am working on \nthis with the hope that it will eventually make it possible to switch \nthe default to the histogram algorithm, for the benefit of many git \nusers. I see no point for this patch if it is not integrated in your \ntree. It is a gift to you and to the git community: if the gift is to be \ndeclined, I'd rather not spend time crafting it.\n\nConcerning the form, I feel obliged to let you know that from my \ncultural standpoint, your reply reads rather aggressive. Specifically, \nthe sentence \"Do not ever say this here.\" reads menacing to me, and the \nuse of all caps later on in your reply reads aggressive to me. I'm doing \nmy best to assume that it is not be the attitude you wanted to convey \nand hope that you can receive this feedback gratefully.\n\n>> +\t*opt &= ~XDF_NEED_MINIMAL;\n>> +\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n>> +\t*opt |= value;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>>   static int is_a_rev(const char *name)\n>>   {\n>>   \tstruct object_id oid;\n>> @@ -908,13 +941,16 @@ int cmd_blame(int argc,\n>>   \t\tOPT_BIT('f', \"show-name\", &output_option, N_(\"show original filename (Default: auto)\"), OUTPUT_SHOW_NAME),\n>>   \t\tOPT_BIT('n', \"show-number\", &output_option, N_(\"show original linenumber (Default: off)\"), OUTPUT_SHOW_NUMBER),\n>>   \t\tOPT_BIT('p', \"porcelain\", &output_option, N_(\"show in a format designed for machine consumption\"), OUTPUT_PORCELAIN),\n>> -\t\tOPT_BIT(0, \"line-porcelain\", &output_option, N_(\"show porcelain format with per-line commit information\"), OUTPUT_PORCELAIN|OUTPUT_LINE_PORCELAIN),\n>> +\t\tOPT_BIT(0, \"line-porcelain\", &output_option, N_(\"show porcelain format with per-line commit information\"), OUTPUT_PORCELAIN | OUTPUT_LINE_PORCELAIN),\n> WHY?\n\nIn an attempt to conform to the coding style of this project, I ran \n`make style`, which generated this change. Given that it is on a line I \nhadn't touched, I pondered on whether to include it in my patch or not. \nIn the past, I had submitted a patch which fixed a formatting issue in \nthe documentation in passing, together with content changes further \ndown, and you had been supportive of this:\n\nhttps://lore.kernel.org/git/xmqq1qaeqtw7.fsf@gitster.g/\n\nSo I decided to include it this time as well. I am happy to remove it if \nyou prefer not to have it.\n\nI thank you for the rest of your comments and will take them into \naccount for a new version of this patch, pending the discussion above.\n\nBest wishes,\n\nAntonin\n\n"},{"id":"529447","messageId":"xmqqfrbay8kw.fsf@gitster.g","threadId":"64356","inReplyTo":"d59a2f97-1a69-44f6-924e-7419e36329a0@delpeuch.eu","subject":"Re: [PATCH] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-22T20:39:43Z","receivedAt":"2025-10-22T20:39:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antonin Delpeuch <antonin@delpeuch.eu> writes:\n\n> On 20/10/2025 18:05, Junio C Hamano wrote:\n>\n>>>      If the general idea of this patch is judged worthwhile, I would be happy\n>>>      to add tests to demonstrate the impact of the diff algorithm on blame\n>>>      output.\n>> Do not ever say this here.\n>>\n>> I've seen from time to time people ask \"I am thinking of doing this;\n>> will a patch be accepted?  If so, I'll work on it.\" before showing\n>> any work, and my response always has been:\n>>\n>>   (1) We don't know how useful and interesting your contribution would\n>>       be for our audience, until we see it; and\n>>\n>>   (2) If you truly believe in your work (find it useful, find writing\n>>       it fun, etc.), that would be incentive enough for you to work\n>>       on it, whether or not the result will land in my tree.  You\n>>       should instead aim for something so brilliant that we would\n>>       come to you begging for your permission to include it in our\n>>       project.\n>\n> I am surprised by your reaction here, both by its substance and form.\n\nYeah, after sending it out, I realized that the canned response\nabove was not fitting to this exact instance.  I overreacted\nprimarily because what I saw everything before that part was\nindication of a great new contributor, which made my dissapointment\nto see the dreaded \"I will do this if this is accepted\" even worse.\n\nYour \"this one lacks tests\" is a bit different from what we\nsometimes see on this list that I react with the above canned\nresponse, which is \"I want to do this great thing.  If you promise\nyou will accept this change, I'll work on it\" without showing any\ndetailed design or code.  It is more like \"I know we need test but I\nhave shown the main part of the change.  Am I going in the right\ndirection?\"\n\nYou certainly didn't deserve the above response.  Sorry about that.\n\n"},{"id":"529508","messageId":"0d6019c7-5e73-4195-b5d2-b43f2cb6399d@gmail.com","threadId":"64356","inReplyTo":"pull.2075.git.git.1760972162827.gitgitgadget@gmail.com","subject":"Re: [PATCH] blame: make diff algorithm configurable","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-10-23T16:03:23Z","receivedAt":"2025-10-23T16:03:27Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Antonin\n\nOn 20/10/2025 15:56, Antonin Delpeuch via GitGitGadget wrote:\n> From: Antonin Delpeuch <antonin@delpeuch.eu>\n> \n> The diff algorithm used in 'git-blame(1)' can be configured using the\n> `--diff-algorithm` option or the `diff.algorithm` config variable.\n> Myers diff remains the default.\n\nI think this sounds like a reasonable thing to do, although it is \ntechnically a breaking change.\n\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 2703820258..177b606e81 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n>   \t\t}\n>   \t}\n>   \n> +\tif (!strcmp(var, \"diff.algorithm\")) {\n> +\t\tlong diff_algorithm;\n> +\t\tif (!value)\n> +\t\t\treturn config_error_nonbool(var);\n> +\t\tdiff_algorithm = parse_algorithm_value(value);\n> +\t\tif (diff_algorithm < 0)\n> +\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n> +\t\t\t\t     var, value);\n> +\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n\nI think this should be\n\n\txdl_opts &= ~(XDF_DIFF_ALGORITHM_MASK | XDF_NEED_MINIMAL);\n\nas you have below for the option parsing.\n> +\t\txdl_opts |= diff_algorithm;\n> +\t\treturn 0;\n> +\t}\n> +\n>   \tif (git_diff_heuristic_config(var, value, cb) < 0)\n>   \t\treturn -1;\n>   \tif (userdiff_config(var, value) < 0)\n> @@ -824,6 +837,26 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n>   \treturn 0;\n>   }\n>   \n> +static int blame_diff_algorithm_callback(const struct option *option,\n> +\t\t\t\t\t const char *arg, int unset)\n> +{\n> +\tint *opt = option->value;\n> +\tlong value = parse_algorithm_value(arg);\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\n> +\tif (value < 0)\n> +\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n> +\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n> +\n> +\t// ignore any previous --minimal setting, following git-diff's behavior\n\nStyle - oneline comments should look like\n\n\t/* comment */\n\n> +\t*opt &= ~XDF_NEED_MINIMAL;\n> +\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n> +\t*opt |= value;\n\n\"git blame\" also has a \"--minimal\" option which now needs to clear the \ndiff algorithm when it sets the minimal flag.\n\nThanks\n\nPhillip\n"},{"id":"529789","messageId":"pull.2075.v2.git.git.1761658643278.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.git.git.1760972162827.gitgitgadget@gmail.com","subject":"[PATCH v2] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-10-28T13:37:23Z","receivedAt":"2025-10-28T13:37:26Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe diff algorithm used in 'git-blame(1)' is set to 'myers',\nwithout the possibility to change it aside from the `--minimal` option.\n\nThere has been long-standing interest in changing the default diff\nalgorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\nfor taking some steps towards that:\n\nhttps://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n\nAs a preparation for this move, it is worth making sure that the diff\nalgorithm is configurable where useful.\n\nMake it configurable in the `git-blame(1)` command by introducing the\n`--diff-algorithm` option and make honor the `diff.algorithm` config\nvariable. Keep Myers diff as the default.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n    blame: make diff algorithm configurable\n    \n    Changes since v1:\n    \n     * add tests\n     * ignore --diff-algorithm when it is provided before --minimal\n     * improve patch description\n     * remove duplication of documentation sections\n     * style improvements\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v2\nPull-Request: https://github.com/git/git/pull/2075\n\nRange-diff vs v1:\n\n 1:  32b59d0204 ! 1:  107f51620b blame: make diff algorithm configurable\n     @@ Metadata\n       ## Commit message ##\n          blame: make diff algorithm configurable\n      \n     -    The diff algorithm used in 'git-blame(1)' can be configured using the\n     -    `--diff-algorithm` option or the `diff.algorithm` config variable.\n     -    Myers diff remains the default.\n     +    The diff algorithm used in 'git-blame(1)' is set to 'myers',\n     +    without the possibility to change it aside from the `--minimal` option.\n     +\n     +    There has been long-standing interest in changing the default diff\n     +    algorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\n     +    for taking some steps towards that:\n     +\n     +    https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n     +\n     +    As a preparation for this move, it is worth making sure that the diff\n     +    algorithm is configurable where useful.\n     +\n     +    Make it configurable in the `git-blame(1)` command by introducing the\n     +    `--diff-algorithm` option and make honor the `diff.algorithm` config\n     +    variable. Keep Myers diff as the default.\n      \n          Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n      \n     - ## Documentation/git-blame.adoc ##\n     -@@ Documentation/git-blame.adoc: include::blame-options.adoc[]\n     - \tIgnore whitespace when comparing the parent's version and\n     - \tthe child's to find where the lines came from.\n     - \n     + ## Documentation/diff-algorithm-option.adoc (new) ##\n     +@@\n      +`--diff-algorithm=(patience|minimal|histogram|myers)`::\n      +\tChoose a diff algorithm. The variants are as follows:\n      ++\n     @@ Documentation/git-blame.adoc: include::blame-options.adoc[]\n      +For instance, if you configured the `diff.algorithm` variable to a\n      +non-default value and want to use the default one, then you\n      +have to use `--diff-algorithm=default` option.\n     +\n     + ## Documentation/diff-options.adoc ##\n     +@@ Documentation/diff-options.adoc: and starts with _<text>_, this algorithm attempts to prevent it from\n     + appearing as a deletion or addition in the output. It uses the \"patience\n     + diff\" algorithm internally.\n     + \n     +-`--diff-algorithm=(patience|minimal|histogram|myers)`::\n     +-\tChoose a diff algorithm. The variants are as follows:\n     +-+\n     +---\n     +-   `default`;;\n     +-   `myers`;;\n     +-\tThe basic greedy diff algorithm. Currently, this is the default.\n     +-   `minimal`;;\n     +-\tSpend extra time to make sure the smallest possible diff is\n     +-\tproduced.\n     +-   `patience`;;\n     +-\tUse \"patience diff\" algorithm when generating patches.\n     +-   `histogram`;;\n     +-\tThis algorithm extends the patience algorithm to \"support\n     +-\tlow-occurrence common elements\".\n     +---\n     +-+\n     +-For instance, if you configured the `diff.algorithm` variable to a\n     +-non-default value and want to use the default one, then you\n     +-have to use `--diff-algorithm=default` option.\n     ++include::diff-algorithm-option.adoc[]\n     + \n     + `--stat[=<width>[,<name-width>[,<count>]]]`::\n     + \tGenerate a diffstat. By default, as much space as necessary\n     +\n     + ## Documentation/git-blame.adoc ##\n     +@@ Documentation/git-blame.adoc: include::blame-options.adoc[]\n     + \tIgnore whitespace when comparing the parent's version and\n     + \tthe child's to find where the lines came from.\n     + \n     ++include::diff-algorithm-option.adoc[]\n      +\n       --abbrev=<n>::\n       \tInstead of using the default 7+1 hexadecimal digits as the\n     @@ builtin/blame.c: static int blame_move_callback(const struct option *option, con\n       \treturn 0;\n       }\n       \n     ++static int blame_diff_algorithm_minimal(const struct option *option,\n     ++\t\t\t\t\tconst char *arg, int unset)\n     ++{\n     ++\tint *opt = option->value;\n     ++\n     ++\tBUG_ON_OPT_NEG(unset);\n     ++\tBUG_ON_OPT_ARG(arg);\n     ++\n     ++\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n     ++\t*opt |= XDF_NEED_MINIMAL;\n     ++\n     ++\treturn 0;\n     ++}\n     ++\n      +static int blame_diff_algorithm_callback(const struct option *option,\n      +\t\t\t\t\t const char *arg, int unset)\n      +{\n     @@ builtin/blame.c: static int blame_move_callback(const struct option *option, con\n      +\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n      +\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n      +\n     -+\t// ignore any previous --minimal setting, following git-diff's behavior\n     -+\t*opt &= ~XDF_NEED_MINIMAL;\n     -+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n     ++\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n      +\t*opt |= value;\n      +\n      +\treturn 0;\n     @@ builtin/blame.c: static int blame_move_callback(const struct option *option, con\n       {\n       \tstruct object_id oid;\n      @@ builtin/blame.c: int cmd_blame(int argc,\n     - \t\tOPT_BIT('f', \"show-name\", &output_option, N_(\"show original filename (Default: auto)\"), OUTPUT_SHOW_NAME),\n     - \t\tOPT_BIT('n', \"show-number\", &output_option, N_(\"show original linenumber (Default: off)\"), OUTPUT_SHOW_NUMBER),\n     - \t\tOPT_BIT('p', \"porcelain\", &output_option, N_(\"show in a format designed for machine consumption\"), OUTPUT_PORCELAIN),\n     --\t\tOPT_BIT(0, \"line-porcelain\", &output_option, N_(\"show porcelain format with per-line commit information\"), OUTPUT_PORCELAIN|OUTPUT_LINE_PORCELAIN),\n     -+\t\tOPT_BIT(0, \"line-porcelain\", &output_option, N_(\"show porcelain format with per-line commit information\"), OUTPUT_PORCELAIN | OUTPUT_LINE_PORCELAIN),\n     - \t\tOPT_BIT('c', NULL, &output_option, N_(\"use the same output mode as git-annotate (Default: off)\"), OUTPUT_ANNOTATE_COMPAT),\n     - \t\tOPT_BIT('t', NULL, &output_option, N_(\"show raw timestamp (Default: off)\"), OUTPUT_RAW_TIMESTAMP),\n     - \t\tOPT_BIT('l', NULL, &output_option, N_(\"show long commit SHA1 (Default: off)\"), OUTPUT_LONG_OBJECT_NAME),\n       \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n       \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n       \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n     @@ builtin/blame.c: int cmd_blame(int argc,\n       \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n       \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n       \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n     + \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n     ++\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n     ++\t\t\t       N_(\"spend extra cycles to find better match\"),\n     ++\t\t\t       PARSE_OPT_NONEG | PARSE_OPT_NOARG,\n     ++\t\t\t       blame_diff_algorithm_minimal),\n     + \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n     + \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n     + \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n     +\n     + ## t/meson.build ##\n     +@@ t/meson.build: integration_tests = [\n     +   't8012-blame-colors.sh',\n     +   't8013-blame-ignore-revs.sh',\n     +   't8014-blame-ignore-fuzzy.sh',\n     ++  't8015-blame-diff-algorithm.sh',\n     +   't8020-last-modified.sh',\n     +   't9001-send-email.sh',\n     +   't9002-column.sh',\n     +\n     + ## t/t8015-blame-diff-algorithm.sh (new) ##\n     +@@\n     ++#!/bin/sh\n     ++\n     ++test_description='git blame with specific diff algorithm'\n     ++\n     ++. ./test-lib.sh\n     ++\n     ++test_expect_success setup '\n     ++\tcat >file.c <<-\\EOF &&\n     ++\tint f(int x, int y)\n     ++\t{\n     ++\t\tif (x == 0)\n     ++\t\t{\n     ++\t\t\treturn y;\n     ++\t\t}\n     ++\t\treturn x;\n     ++\t}\n     ++\n     ++\tint g(size_t u)\n     ++\t{\n     ++\t\twhile (u < 30)\n     ++\t\t{\n     ++\t\t\tu++;\n     ++\t\t}\n     ++\t\treturn u;\n     ++\t}\n     ++\tEOF\n     ++\ttest_write_lines x x x x >file.txt &&\n     ++\tgit add file.c file.txt &&\n     ++\tGIT_AUTHOR_NAME=Initial git commit -m Initial &&\n     ++\n     ++\tcat >file.c <<-\\EOF &&\n     ++\tint g(size_t u)\n     ++\t{\n     ++\t\twhile (u < 30)\n     ++\t\t{\n     ++\t\t\tu++;\n     ++\t\t}\n     ++\t\treturn u;\n     ++\t}\n     ++\n     ++\tint h(int x, int y, int z)\n     ++\t{\n     ++\t\tif (z == 0)\n     ++\t\t{\n     ++\t\t\treturn x;\n     ++\t\t}\n     ++\t\treturn y;\n     ++\t}\n     ++\tEOF\n     ++\ttest_write_lines x x x A B C D x E F G >file.txt &&\n     ++\tgit add file.c file.txt &&\n     ++\tGIT_AUTHOR_NAME=Second git commit -m Second\n     ++'\n     ++\n     ++test_expect_success 'blame uses Myers diff algorithm by default for now' '\n     ++\tcat >expected <<-\\EOF &&\n     ++\tSecond\n     ++\tInitial\n     ++\tSecond\n     ++\tInitial\n     ++\tSecond\n     ++\tInitial\n     ++\tSecond\n     ++\tInitial\n     ++\tInitial\n     ++\tSecond\n     ++\tInitial\n     ++\tSecond\n     ++\tInitial\n     ++\tSecond\n     ++\tInitial\n     ++\tSecond\n     ++\tInitial\n     ++\tEOF\n     ++\n     ++\t# git blame file.c | grep --only-matching -e Initial -e Second > actual &&\n     ++\t# test_cmp expected actual\n     ++\techo goo\n     ++'\n     ++\n     ++test_expect_success 'blame honors --diff-algorithm option' '\n     ++\tcat >expected <<-\\EOF &&\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tEOF\n     ++\n     ++\tgit blame file.c --diff-algorithm=histogram | \\\n     ++\t\tgrep --only-matching -e Initial -e Second > actual &&\n     ++\ttest_cmp expected actual\n     ++'\n     ++\n     ++test_expect_success 'blame honors diff.algorithm config variable' '\n     ++\tcat >expected <<-\\EOF &&\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tEOF\n     ++\n     ++\tgit config diff.algorithm histogram &&\n     ++\tgit blame file.c | \\\n     ++\t\tgrep --only-matching -e Initial -e Second > actual &&\n     ++\ttest_cmp expected actual\n     ++'\n     ++\n     ++test_expect_success 'blame honors --minimal option' '\n     ++\tcat >expected <<-\\EOF &&\n     ++\tInitial\n     ++\tInitial\n     ++\tInitial\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tInitial\n     ++\tSecond\n     ++\tSecond\n     ++\tSecond\n     ++\tEOF\n     ++\n     ++\tgit blame file.txt --minimal | \\\n     ++\t\tgrep --only-matching -e Initial -e Second > actual &&\n     ++\ttest_cmp expected actual\n     ++'\n     ++\n     ++test_done\n\n\n Documentation/diff-algorithm-option.adoc |  20 +++\n Documentation/diff-options.adoc          |  21 +---\n Documentation/git-blame.adoc             |   2 +\n builtin/blame.c                          |  52 ++++++++\n t/meson.build                            |   1 +\n t/t8015-blame-diff-algorithm.sh          | 154 +++++++++++++++++++++++\n 6 files changed, 230 insertions(+), 20 deletions(-)\n create mode 100644 Documentation/diff-algorithm-option.adoc\n create mode 100755 t/t8015-blame-diff-algorithm.sh\n\ndiff --git a/Documentation/diff-algorithm-option.adoc b/Documentation/diff-algorithm-option.adoc\nnew file mode 100644\nindex 0000000000..8e3a0b63d7\n--- /dev/null\n+++ b/Documentation/diff-algorithm-option.adoc\n@@ -0,0 +1,20 @@\n+`--diff-algorithm=(patience|minimal|histogram|myers)`::\n+\tChoose a diff algorithm. The variants are as follows:\n++\n+--\n+   `default`;;\n+   `myers`;;\n+\tThe basic greedy diff algorithm. Currently, this is the default.\n+   `minimal`;;\n+\tSpend extra time to make sure the smallest possible diff is\n+\tproduced.\n+   `patience`;;\n+\tUse \"patience diff\" algorithm when generating patches.\n+   `histogram`;;\n+\tThis algorithm extends the patience algorithm to \"support\n+\tlow-occurrence common elements\".\n+--\n++\n+For instance, if you configured the `diff.algorithm` variable to a\n+non-default value and want to use the default one, then you\n+have to use `--diff-algorithm=default` option.\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex ae31520f7f..9cdad6f72a 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -197,26 +197,7 @@ and starts with _<text>_, this algorithm attempts to prevent it from\n appearing as a deletion or addition in the output. It uses the \"patience\n diff\" algorithm internally.\n \n-`--diff-algorithm=(patience|minimal|histogram|myers)`::\n-\tChoose a diff algorithm. The variants are as follows:\n-+\n---\n-   `default`;;\n-   `myers`;;\n-\tThe basic greedy diff algorithm. Currently, this is the default.\n-   `minimal`;;\n-\tSpend extra time to make sure the smallest possible diff is\n-\tproduced.\n-   `patience`;;\n-\tUse \"patience diff\" algorithm when generating patches.\n-   `histogram`;;\n-\tThis algorithm extends the patience algorithm to \"support\n-\tlow-occurrence common elements\".\n---\n-+\n-For instance, if you configured the `diff.algorithm` variable to a\n-non-default value and want to use the default one, then you\n-have to use `--diff-algorithm=default` option.\n+include::diff-algorithm-option.adoc[]\n \n `--stat[=<width>[,<name-width>[,<count>]]]`::\n \tGenerate a diffstat. By default, as much space as necessary\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex e438d28625..adcbb6f5dc 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -85,6 +85,8 @@ include::blame-options.adoc[]\n \tIgnore whitespace when comparing the parent's version and\n \tthe child's to find where the lines came from.\n \n+include::diff-algorithm-option.adoc[]\n+\n --abbrev=<n>::\n \tInstead of using the default 7+1 hexadecimal digits as the\n \tabbreviated object name, use <m>+1 digits, where <m> is at\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 2703820258..eb0ab71dba 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n \t\t}\n \t}\n \n+\tif (!strcmp(var, \"diff.algorithm\")) {\n+\t\tlong diff_algorithm;\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tdiff_algorithm = parse_algorithm_value(value);\n+\t\tif (diff_algorithm < 0)\n+\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n+\t\t\t\t     var, value);\n+\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t\txdl_opts |= diff_algorithm;\n+\t\treturn 0;\n+\t}\n+\n \tif (git_diff_heuristic_config(var, value, cb) < 0)\n \t\treturn -1;\n \tif (userdiff_config(var, value) < 0)\n@@ -824,6 +837,38 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n \treturn 0;\n }\n \n+static int blame_diff_algorithm_minimal(const struct option *option,\n+\t\t\t\t\tconst char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(arg);\n+\n+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t*opt |= XDF_NEED_MINIMAL;\n+\n+\treturn 0;\n+}\n+\n+static int blame_diff_algorithm_callback(const struct option *option,\n+\t\t\t\t\t const char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\tlong value = parse_algorithm_value(arg);\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tif (value < 0)\n+\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n+\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n+\n+\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n+\t*opt |= value;\n+\n+\treturn 0;\n+}\n+\n static int is_a_rev(const char *name)\n {\n \tstruct object_id oid;\n@@ -915,10 +960,17 @@ int cmd_blame(int argc,\n \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n+\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n+\t\t\t       N_(\"choose a diff algorithm\"),\n+\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n+\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n+\t\t\t       N_(\"spend extra cycles to find better match\"),\n+\t\t\t       PARSE_OPT_NONEG | PARSE_OPT_NOARG,\n+\t\t\t       blame_diff_algorithm_minimal),\n \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\ndiff --git a/t/meson.build b/t/meson.build\nindex 401b24e50e..9f2fe7af8b 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -955,6 +955,7 @@ integration_tests = [\n   't8012-blame-colors.sh',\n   't8013-blame-ignore-revs.sh',\n   't8014-blame-ignore-fuzzy.sh',\n+  't8015-blame-diff-algorithm.sh',\n   't8020-last-modified.sh',\n   't9001-send-email.sh',\n   't9002-column.sh',\ndiff --git a/t/t8015-blame-diff-algorithm.sh b/t/t8015-blame-diff-algorithm.sh\nnew file mode 100755\nindex 0000000000..43996df177\n--- /dev/null\n+++ b/t/t8015-blame-diff-algorithm.sh\n@@ -0,0 +1,154 @@\n+#!/bin/sh\n+\n+test_description='git blame with specific diff algorithm'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tcat >file.c <<-\\EOF &&\n+\tint f(int x, int y)\n+\t{\n+\t\tif (x == 0)\n+\t\t{\n+\t\t\treturn y;\n+\t\t}\n+\t\treturn x;\n+\t}\n+\n+\tint g(size_t u)\n+\t{\n+\t\twhile (u < 30)\n+\t\t{\n+\t\t\tu++;\n+\t\t}\n+\t\treturn u;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x x >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Initial git commit -m Initial &&\n+\n+\tcat >file.c <<-\\EOF &&\n+\tint g(size_t u)\n+\t{\n+\t\twhile (u < 30)\n+\t\t{\n+\t\t\tu++;\n+\t\t}\n+\t\treturn u;\n+\t}\n+\n+\tint h(int x, int y, int z)\n+\t{\n+\t\tif (z == 0)\n+\t\t{\n+\t\t\treturn x;\n+\t\t}\n+\t\treturn y;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x A B C D x E F G >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Second git commit -m Second\n+'\n+\n+test_expect_success 'blame uses Myers diff algorithm by default for now' '\n+\tcat >expected <<-\\EOF &&\n+\tSecond\n+\tInitial\n+\tSecond\n+\tInitial\n+\tSecond\n+\tInitial\n+\tSecond\n+\tInitial\n+\tInitial\n+\tSecond\n+\tInitial\n+\tSecond\n+\tInitial\n+\tSecond\n+\tInitial\n+\tSecond\n+\tInitial\n+\tEOF\n+\n+\t# git blame file.c | grep --only-matching -e Initial -e Second > actual &&\n+\t# test_cmp expected actual\n+\techo goo\n+'\n+\n+test_expect_success 'blame honors --diff-algorithm option' '\n+\tcat >expected <<-\\EOF &&\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tEOF\n+\n+\tgit blame file.c --diff-algorithm=histogram | \\\n+\t\tgrep --only-matching -e Initial -e Second > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors diff.algorithm config variable' '\n+\tcat >expected <<-\\EOF &&\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tInitial\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tEOF\n+\n+\tgit config diff.algorithm histogram &&\n+\tgit blame file.c | \\\n+\t\tgrep --only-matching -e Initial -e Second > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors --minimal option' '\n+\tcat >expected <<-\\EOF &&\n+\tInitial\n+\tInitial\n+\tInitial\n+\tSecond\n+\tSecond\n+\tSecond\n+\tSecond\n+\tInitial\n+\tSecond\n+\tSecond\n+\tSecond\n+\tEOF\n+\n+\tgit blame file.txt --minimal | \\\n+\t\tgrep --only-matching -e Initial -e Second > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n\nbase-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\n-- \ngitgitgadget\n"},{"id":"529793","messageId":"xmqqjz0fdpa3.fsf@gitster.g","threadId":"64356","inReplyTo":"pull.2075.v2.git.git.1761658643278.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-28T15:22:44Z","receivedAt":"2025-10-28T15:22:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Antonin Delpeuch via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Antonin Delpeuch <antonin@delpeuch.eu>\n>\n> The diff algorithm used in 'git-blame(1)' is set to 'myers',\n> without the possibility to change it aside from the `--minimal` option.\n\nHmph.  It is very unfortunate that we had --minimal already.  We\nshould have done --diff-algorithm=<which> instead, but that is way\ntoo late.\n\n> There has been long-standing interest in changing the default diff\n> algorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\n> for taking some steps towards that:\n>\n> https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n\nMicronit.  I think the reference to 3.0 only about potentially\nbreaking backward compatibility by making the family of diff-*\nplumbing commands ignore diff.algorithm configuration, and other\nusability changes like this one are fair game without having to wait\nfor 3.0 boundary (the plumbing commands do ignore the configuration\nalready, so there is nothing we have to wait 3.0 before doing).\n\n>     Changes since v1:\n>     \n>      * add tests\n>      * ignore --diff-algorithm when it is provided before --minimal\n\nSensible.\n\nI presume the reverse is true, i.e. giving \"--minimal\" and then\n\"--diff-algorithm=histogram\" in this order would make \"histogram\"\nsurvive, in other words, the usual \"last one wins\" rule is applied?\n\n> +static int blame_diff_algorithm_minimal(const struct option *option,\n> +\t\t\t\t\tconst char *arg, int unset)\n> +{\n> +\tint *opt = option->value;\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\tBUG_ON_OPT_ARG(arg);\n> +\n> +\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n> +\t*opt |= XDF_NEED_MINIMAL;\n> +\n> +\treturn 0;\n> +}\n\nThis and diff.c:diff_opt_diff_algorithm_no_arg(), which I think is\nthe original from which this was copied from, look somewhat\ndifferent, but this can afford to be simpler, as it does not have to\nparse \"--histogram\", \"--patience\", etc., as independent command line\noptions.  OK.\n\n> +static int blame_diff_algorithm_callback(const struct option *option,\n> +\t\t\t\t\t const char *arg, int unset)\n> +{\n> +\tint *opt = option->value;\n> +\tlong value = parse_algorithm_value(arg);\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\n> +\tif (value < 0)\n> +\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n> +\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n> +\n> +\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n> +\t*opt |= value;\n> +\n> +\treturn 0;\n> +}\n\nQuite straight-forward and sensible.\n\n>  static int is_a_rev(const char *name)\n>  {\n>  \tstruct object_id oid;\n> @@ -915,10 +960,17 @@ int cmd_blame(int argc,\n>  \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n>  \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n>  \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n> +\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n> +\t\t\t       N_(\"choose a diff algorithm\"),\n> +\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n>  \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n>  \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n>  \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n>  \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n> +\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n> +\t\t\t       N_(\"spend extra cycles to find better match\"),\n> +\t\t\t       PARSE_OPT_NONEG | PARSE_OPT_NOARG,\n> +\t\t\t       blame_diff_algorithm_minimal),\n>  \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n\nThis OPT_BIT() can stay here?  I thought parse_options_check() was\ncapable of detecting duplicated long-form commands as programming\nerror, but apparently it does not.  (#leftoverbits) We should look\ninto teaching parse_options_check() to check duplicated option\nnames.\n\n>  \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n>  \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n> ...\n> +test_expect_success 'blame honors --minimal option' '\n> +\tcat >expected <<-\\EOF &&\n> +\tInitial\n> +\tInitial\n> +\tInitial\n> +\tSecond\n> +\tSecond\n> +\tSecond\n> +\tSecond\n> +\tInitial\n> +\tSecond\n> +\tSecond\n> +\tSecond\n> +\tEOF\n> +\n> +\tgit blame file.txt --minimal | \\\n> +\t\tgrep --only-matching -e Initial -e Second > actual &&\n> +\ttest_cmp expected actual\n> +'\n\nDo we need to test combination of configuration variables and\ncommand line options (to verify that options trump configuration),\nor two command line options (to verify that the last one wins)?\n\nWhen xdiff/ part of the system gets improved, the above expected\npatterns may have to change, these tests may fail.  Whoever updates\nthe diff algorithm to cause such a failure has to tell between a\ngenuine _bug_ in their update to diff implementation and the test\nexpecting a suboptimal result based on the behaviour of the diff\nalgorithm before their improvement.  And for that, they need to\ndebug these tests.  But I suspect that these tests will probably be\nvery difficult to debug, as it is almost impossible to see which\nline in the original each of these lines correspond to.\n\nI guess that's inevitable, and we'll cross that bridge when it\nbecomes necessary.\n\nThanks, will queue, but I do find the leftover --minimal bit\ndisturbing.\n\n\n"},{"id":"529796","messageId":"362c7dc4-c35f-440a-ae20-e1d06e183fce@delpeuch.eu","threadId":"64356","inReplyTo":"xmqqjz0fdpa3.fsf@gitster.g","subject":"Re: [PATCH v2] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch","fromEmail":"antonin@delpeuch.eu","sentAt":"2025-10-28T16:00:40Z","receivedAt":"2025-10-28T16:07:30Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"Hi Junio,\n\nOn 28/10/2025 16:22, Junio C Hamano wrote:\n>>      Changes since v1:\n>>      \n>>       * add tests\n>>       * ignore --diff-algorithm when it is provided before --minimal\n> Sensible.\n>\n> I presume the reverse is true, i.e. giving \"--minimal\" and then\n> \"--diff-algorithm=histogram\" in this order would make \"histogram\"\n> survive, in other words, the usual \"last one wins\" rule is applied?\nYes, that was already the case in the version 1 of this patch. Philipp \nnoticed that it was only true in one direction, so this fixes it.\n>>   static int is_a_rev(const char *name)\n>>   {\n>>   \tstruct object_id oid;\n>> @@ -915,10 +960,17 @@ int cmd_blame(int argc,\n>>   \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n>>   \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n>>   \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n>> +\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n>> +\t\t\t       N_(\"choose a diff algorithm\"),\n>> +\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n>>   \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n>>   \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n>>   \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n>>   \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n>> +\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n>> +\t\t\t       N_(\"spend extra cycles to find better match\"),\n>> +\t\t\t       PARSE_OPT_NONEG | PARSE_OPT_NOARG,\n>> +\t\t\t       blame_diff_algorithm_minimal),\n>>   \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n> This OPT_BIT() can stay here?  I thought parse_options_check() was\n> capable of detecting duplicated long-form commands as programming\n> error, but apparently it does not.  (#leftoverbits) We should look\n> into teaching parse_options_check() to check duplicated option\n> names.\n\nOops, that's an oversight on my part indeed. I'll fix it.\n\n>\n>>   \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n>>   \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n>> ...\n>> +test_expect_success 'blame honors --minimal option' '\n>> +\tcat >expected <<-\\EOF &&\n>> +\tInitial\n>> +\tInitial\n>> +\tInitial\n>> +\tSecond\n>> +\tSecond\n>> +\tSecond\n>> +\tSecond\n>> +\tInitial\n>> +\tSecond\n>> +\tSecond\n>> +\tSecond\n>> +\tEOF\n>> +\n>> +\tgit blame file.txt --minimal | \\\n>> +\t\tgrep --only-matching -e Initial -e Second > actual &&\n>> +\ttest_cmp expected actual\n>> +'\n> Do we need to test combination of configuration variables and\n> command line options (to verify that options trump configuration),\n> or two command line options (to verify that the last one wins)?\nIt's easy enough to add such tests, I can add a few more.\n>\n> When xdiff/ part of the system gets improved, the above expected\n> patterns may have to change, these tests may fail.  Whoever updates\n> the diff algorithm to cause such a failure has to tell between a\n> genuine _bug_ in their update to diff implementation and the test\n> expecting a suboptimal result based on the behaviour of the diff\n> algorithm before their improvement.  And for that, they need to\n> debug these tests.  But I suspect that these tests will probably be\n> very difficult to debug, as it is almost impossible to see which\n> line in the original each of these lines correspond to.\n\nI'm happy to make the tests a bit clearer by including the lines in the \nexpected output.\n\nI'll submit a new version with those changes.\n\nBest,\n\nAntonin\n\n"},{"id":"529835","messageId":"pull.2075.v3.git.git.1761686060477.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v2.git.git.1761658643278.gitgitgadget@gmail.com","subject":"[PATCH v3] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-10-28T21:14:20Z","receivedAt":"2025-10-28T21:14:25Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe diff algorithm used in 'git-blame(1)' is set to 'myers',\nwithout the possibility to change it aside from the `--minimal` option.\n\nThere has been long-standing interest in changing the default diff\nalgorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\nfor taking some steps towards that:\n\nhttps://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n\nAs a preparation for this move, it is worth making sure that the diff\nalgorithm is configurable where useful.\n\nMake it configurable in the `git-blame(1)` command by introducing the\n`--diff-algorithm` option and make honor the `diff.algorithm` config\nvariable. Keep Myers diff as the default.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n    blame: make diff algorithm configurable\n    \n    Changes since v1:\n    \n     * add tests\n     * ignore --diff-algorithm when it is provided before --minimal\n     * improve patch description\n     * remove duplication of documentation sections\n     * style improvements\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v3\nPull-Request: https://github.com/git/git/pull/2075\n\nRange-diff vs v2:\n\n 1:  107f51620b ! 1:  b8bdb03516 blame: make diff algorithm configurable\n     @@ builtin/blame.c: int cmd_blame(int argc,\n       \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n       \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n       \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n     +-\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n      +\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n      +\t\t\t       N_(\"spend extra cycles to find better match\"),\n      +\t\t\t       PARSE_OPT_NONEG | PARSE_OPT_NOARG,\n      +\t\t\t       blame_diff_algorithm_minimal),\n     - \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n       \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n       \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n     + \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\n      \n       ## t/meson.build ##\n      @@ t/meson.build: integration_tests = [\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tcat >file.c <<-\\EOF &&\n      +\tint f(int x, int y)\n      +\t{\n     -+\t\tif (x == 0)\n     -+\t\t{\n     -+\t\t\treturn y;\n     -+\t\t}\n     -+\t\treturn x;\n     ++\t  if (x == 0)\n     ++\t  {\n     ++\t    return y;\n     ++\t  }\n     ++\t  return x;\n      +\t}\n      +\n      +\tint g(size_t u)\n      +\t{\n     -+\t\twhile (u < 30)\n     -+\t\t{\n     -+\t\t\tu++;\n     -+\t\t}\n     -+\t\treturn u;\n     ++\t  while (u < 30)\n     ++\t  {\n     ++\t    u++;\n     ++\t  }\n     ++\t  return u;\n      +\t}\n      +\tEOF\n      +\ttest_write_lines x x x x >file.txt &&\n      +\tgit add file.c file.txt &&\n     -+\tGIT_AUTHOR_NAME=Initial git commit -m Initial &&\n     ++\tGIT_AUTHOR_NAME=Commit_1 git commit -m Commit_1 &&\n      +\n      +\tcat >file.c <<-\\EOF &&\n      +\tint g(size_t u)\n      +\t{\n     -+\t\twhile (u < 30)\n     -+\t\t{\n     -+\t\t\tu++;\n     -+\t\t}\n     -+\t\treturn u;\n     ++\t  while (u < 30)\n     ++\t  {\n     ++\t    u++;\n     ++\t  }\n     ++\t  return u;\n      +\t}\n      +\n      +\tint h(int x, int y, int z)\n      +\t{\n     -+\t\tif (z == 0)\n     -+\t\t{\n     -+\t\t\treturn x;\n     -+\t\t}\n     -+\t\treturn y;\n     ++\t  if (z == 0)\n     ++\t  {\n     ++\t    return x;\n     ++\t  }\n     ++\t  return y;\n      +\t}\n      +\tEOF\n      +\ttest_write_lines x x x A B C D x E F G >file.txt &&\n      +\tgit add file.c file.txt &&\n     -+\tGIT_AUTHOR_NAME=Second git commit -m Second\n     ++\tGIT_AUTHOR_NAME=Commit_2 git commit -m Commit_2\n      +'\n      +\n      +test_expect_success 'blame uses Myers diff algorithm by default for now' '\n      +\tcat >expected <<-\\EOF &&\n     -+\tSecond\n     -+\tInitial\n     -+\tSecond\n     -+\tInitial\n     -+\tSecond\n     -+\tInitial\n     -+\tSecond\n     -+\tInitial\n     -+\tInitial\n     -+\tSecond\n     -+\tInitial\n     -+\tSecond\n     -+\tInitial\n     -+\tSecond\n     -+\tInitial\n     -+\tSecond\n     -+\tInitial\n     ++\tCommit_2 int g(size_t u)\n     ++\tCommit_1 {\n     ++\tCommit_2   while (u < 30)\n     ++\tCommit_1   {\n     ++\tCommit_2     u++;\n     ++\tCommit_1   }\n     ++\tCommit_2   return u;\n     ++\tCommit_1 }\n     ++\tCommit_1\n     ++\tCommit_2 int h(int x, int y, int z)\n     ++\tCommit_1 {\n     ++\tCommit_2   if (z == 0)\n     ++\tCommit_1   {\n     ++\tCommit_2     return x;\n     ++\tCommit_1   }\n     ++\tCommit_2   return y;\n     ++\tCommit_1 }\n      +\tEOF\n      +\n     -+\t# git blame file.c | grep --only-matching -e Initial -e Second > actual &&\n     -+\t# test_cmp expected actual\n     -+\techo goo\n     ++\n     ++\tgit blame file.c | \\\n     ++\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n     ++\t\tsed -e \"s/ *$//g\" > actual &&\n     ++\ttest_cmp expected actual\n      +'\n      +\n      +test_expect_success 'blame honors --diff-algorithm option' '\n      +\tcat >expected <<-\\EOF &&\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     ++\tCommit_1 int g(size_t u)\n     ++\tCommit_1 {\n     ++\tCommit_1   while (u < 30)\n     ++\tCommit_1   {\n     ++\tCommit_1     u++;\n     ++\tCommit_1   }\n     ++\tCommit_1   return u;\n     ++\tCommit_1 }\n     ++\tCommit_2\n     ++\tCommit_2 int h(int x, int y, int z)\n     ++\tCommit_2 {\n     ++\tCommit_2   if (z == 0)\n     ++\tCommit_2   {\n     ++\tCommit_2     return x;\n     ++\tCommit_2   }\n     ++\tCommit_2   return y;\n     ++\tCommit_2 }\n      +\tEOF\n      +\n     -+\tgit blame file.c --diff-algorithm=histogram | \\\n     -+\t\tgrep --only-matching -e Initial -e Second > actual &&\n     ++\tgit blame file.c --diff-algorithm histogram | \\\n     ++\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n     ++\t\tsed -e \"s/ *$//g\" > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n      +test_expect_success 'blame honors diff.algorithm config variable' '\n      +\tcat >expected <<-\\EOF &&\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     ++\tCommit_1 int g(size_t u)\n     ++\tCommit_1 {\n     ++\tCommit_1   while (u < 30)\n     ++\tCommit_1   {\n     ++\tCommit_1     u++;\n     ++\tCommit_1   }\n     ++\tCommit_1   return u;\n     ++\tCommit_1 }\n     ++\tCommit_2\n     ++\tCommit_2 int h(int x, int y, int z)\n     ++\tCommit_2 {\n     ++\tCommit_2   if (z == 0)\n     ++\tCommit_2   {\n     ++\tCommit_2     return x;\n     ++\tCommit_2   }\n     ++\tCommit_2   return y;\n     ++\tCommit_2 }\n      +\tEOF\n      +\n      +\tgit config diff.algorithm histogram &&\n      +\tgit blame file.c | \\\n     -+\t\tgrep --only-matching -e Initial -e Second > actual &&\n     ++\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n     ++\t\tsed -e \"s/ *$//g\" > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     ++test_expect_success 'blame gives priority to --diff-algorithm over diff.algorithm' '\n     ++\tcat >expected <<-\\EOF &&\n     ++\tCommit_1 int g(size_t u)\n     ++\tCommit_1 {\n     ++\tCommit_1   while (u < 30)\n     ++\tCommit_1   {\n     ++\tCommit_1     u++;\n     ++\tCommit_1   }\n     ++\tCommit_1   return u;\n     ++\tCommit_1 }\n     ++\tCommit_2\n     ++\tCommit_2 int h(int x, int y, int z)\n     ++\tCommit_2 {\n     ++\tCommit_2   if (z == 0)\n     ++\tCommit_2   {\n     ++\tCommit_2     return x;\n     ++\tCommit_2   }\n     ++\tCommit_2   return y;\n     ++\tCommit_2 }\n     ++\tEOF\n     ++\n     ++\tgit config diff.algorithm myers &&\n     ++\tgit blame file.c --diff-algorithm histogram | \\\n     ++\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n     ++\t\tsed -e \"s/ *$//g\" > actual &&\n     ++\ttest_cmp expected actual\n     ++'\n      +test_expect_success 'blame honors --minimal option' '\n      +\tcat >expected <<-\\EOF &&\n     -+\tInitial\n     -+\tInitial\n     -+\tInitial\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     -+\tInitial\n     -+\tSecond\n     -+\tSecond\n     -+\tSecond\n     ++\tCommit_1 x\n     ++\tCommit_1 x\n     ++\tCommit_1 x\n     ++\tCommit_2 A\n     ++\tCommit_2 B\n     ++\tCommit_2 C\n     ++\tCommit_2 D\n     ++\tCommit_1 x\n     ++\tCommit_2 E\n     ++\tCommit_2 F\n     ++\tCommit_2 G\n      +\tEOF\n      +\n      +\tgit blame file.txt --minimal | \\\n     -+\t\tgrep --only-matching -e Initial -e Second > actual &&\n     ++\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     ++test_expect_success 'blame respects the order of diff options' '\n     ++\tcat >expected <<-\\EOF &&\n     ++\tCommit_1 x\n     ++\tCommit_1 x\n     ++\tCommit_1 x\n     ++\tCommit_2 A\n     ++\tCommit_2 B\n     ++\tCommit_2 C\n     ++\tCommit_2 D\n     ++\tCommit_2 x\n     ++\tCommit_2 E\n     ++\tCommit_2 F\n     ++\tCommit_2 G\n     ++\tEOF\n     ++\n     ++\tgit blame file.txt --minimal --diff-algorithm myers | \\\n     ++\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" > actual &&\n     ++\ttest_cmp expected actual\n     ++'\n     ++\n     ++\n      +test_done\n\n\n Documentation/diff-algorithm-option.adoc |  20 +++\n Documentation/diff-options.adoc          |  21 +--\n Documentation/git-blame.adoc             |   2 +\n builtin/blame.c                          |  53 +++++-\n t/meson.build                            |   1 +\n t/t8015-blame-diff-algorithm.sh          | 206 +++++++++++++++++++++++\n 6 files changed, 282 insertions(+), 21 deletions(-)\n create mode 100644 Documentation/diff-algorithm-option.adoc\n create mode 100755 t/t8015-blame-diff-algorithm.sh\n\ndiff --git a/Documentation/diff-algorithm-option.adoc b/Documentation/diff-algorithm-option.adoc\nnew file mode 100644\nindex 0000000000..8e3a0b63d7\n--- /dev/null\n+++ b/Documentation/diff-algorithm-option.adoc\n@@ -0,0 +1,20 @@\n+`--diff-algorithm=(patience|minimal|histogram|myers)`::\n+\tChoose a diff algorithm. The variants are as follows:\n++\n+--\n+   `default`;;\n+   `myers`;;\n+\tThe basic greedy diff algorithm. Currently, this is the default.\n+   `minimal`;;\n+\tSpend extra time to make sure the smallest possible diff is\n+\tproduced.\n+   `patience`;;\n+\tUse \"patience diff\" algorithm when generating patches.\n+   `histogram`;;\n+\tThis algorithm extends the patience algorithm to \"support\n+\tlow-occurrence common elements\".\n+--\n++\n+For instance, if you configured the `diff.algorithm` variable to a\n+non-default value and want to use the default one, then you\n+have to use `--diff-algorithm=default` option.\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex ae31520f7f..9cdad6f72a 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -197,26 +197,7 @@ and starts with _<text>_, this algorithm attempts to prevent it from\n appearing as a deletion or addition in the output. It uses the \"patience\n diff\" algorithm internally.\n \n-`--diff-algorithm=(patience|minimal|histogram|myers)`::\n-\tChoose a diff algorithm. The variants are as follows:\n-+\n---\n-   `default`;;\n-   `myers`;;\n-\tThe basic greedy diff algorithm. Currently, this is the default.\n-   `minimal`;;\n-\tSpend extra time to make sure the smallest possible diff is\n-\tproduced.\n-   `patience`;;\n-\tUse \"patience diff\" algorithm when generating patches.\n-   `histogram`;;\n-\tThis algorithm extends the patience algorithm to \"support\n-\tlow-occurrence common elements\".\n---\n-+\n-For instance, if you configured the `diff.algorithm` variable to a\n-non-default value and want to use the default one, then you\n-have to use `--diff-algorithm=default` option.\n+include::diff-algorithm-option.adoc[]\n \n `--stat[=<width>[,<name-width>[,<count>]]]`::\n \tGenerate a diffstat. By default, as much space as necessary\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex e438d28625..adcbb6f5dc 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -85,6 +85,8 @@ include::blame-options.adoc[]\n \tIgnore whitespace when comparing the parent's version and\n \tthe child's to find where the lines came from.\n \n+include::diff-algorithm-option.adoc[]\n+\n --abbrev=<n>::\n \tInstead of using the default 7+1 hexadecimal digits as the\n \tabbreviated object name, use <m>+1 digits, where <m> is at\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 2703820258..da4dbdf50a 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n \t\t}\n \t}\n \n+\tif (!strcmp(var, \"diff.algorithm\")) {\n+\t\tlong diff_algorithm;\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tdiff_algorithm = parse_algorithm_value(value);\n+\t\tif (diff_algorithm < 0)\n+\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n+\t\t\t\t     var, value);\n+\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t\txdl_opts |= diff_algorithm;\n+\t\treturn 0;\n+\t}\n+\n \tif (git_diff_heuristic_config(var, value, cb) < 0)\n \t\treturn -1;\n \tif (userdiff_config(var, value) < 0)\n@@ -824,6 +837,38 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n \treturn 0;\n }\n \n+static int blame_diff_algorithm_minimal(const struct option *option,\n+\t\t\t\t\tconst char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(arg);\n+\n+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t*opt |= XDF_NEED_MINIMAL;\n+\n+\treturn 0;\n+}\n+\n+static int blame_diff_algorithm_callback(const struct option *option,\n+\t\t\t\t\t const char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\tlong value = parse_algorithm_value(arg);\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tif (value < 0)\n+\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n+\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n+\n+\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n+\t*opt |= value;\n+\n+\treturn 0;\n+}\n+\n static int is_a_rev(const char *name)\n {\n \tstruct object_id oid;\n@@ -915,11 +960,17 @@ int cmd_blame(int argc,\n \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n+\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n+\t\t\t       N_(\"choose a diff algorithm\"),\n+\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n-\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n+\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n+\t\t\t       N_(\"spend extra cycles to find better match\"),\n+\t\t\t       PARSE_OPT_NONEG | PARSE_OPT_NOARG,\n+\t\t\t       blame_diff_algorithm_minimal),\n \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\ndiff --git a/t/meson.build b/t/meson.build\nindex 401b24e50e..9f2fe7af8b 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -955,6 +955,7 @@ integration_tests = [\n   't8012-blame-colors.sh',\n   't8013-blame-ignore-revs.sh',\n   't8014-blame-ignore-fuzzy.sh',\n+  't8015-blame-diff-algorithm.sh',\n   't8020-last-modified.sh',\n   't9001-send-email.sh',\n   't9002-column.sh',\ndiff --git a/t/t8015-blame-diff-algorithm.sh b/t/t8015-blame-diff-algorithm.sh\nnew file mode 100755\nindex 0000000000..efc4b47ce1\n--- /dev/null\n+++ b/t/t8015-blame-diff-algorithm.sh\n@@ -0,0 +1,206 @@\n+#!/bin/sh\n+\n+test_description='git blame with specific diff algorithm'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tcat >file.c <<-\\EOF &&\n+\tint f(int x, int y)\n+\t{\n+\t  if (x == 0)\n+\t  {\n+\t    return y;\n+\t  }\n+\t  return x;\n+\t}\n+\n+\tint g(size_t u)\n+\t{\n+\t  while (u < 30)\n+\t  {\n+\t    u++;\n+\t  }\n+\t  return u;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x x >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Commit_1 git commit -m Commit_1 &&\n+\n+\tcat >file.c <<-\\EOF &&\n+\tint g(size_t u)\n+\t{\n+\t  while (u < 30)\n+\t  {\n+\t    u++;\n+\t  }\n+\t  return u;\n+\t}\n+\n+\tint h(int x, int y, int z)\n+\t{\n+\t  if (z == 0)\n+\t  {\n+\t    return x;\n+\t  }\n+\t  return y;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x A B C D x E F G >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Commit_2 git commit -m Commit_2\n+'\n+\n+test_expect_success 'blame uses Myers diff algorithm by default for now' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_2 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_2   while (u < 30)\n+\tCommit_1   {\n+\tCommit_2     u++;\n+\tCommit_1   }\n+\tCommit_2   return u;\n+\tCommit_1 }\n+\tCommit_1\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_1 {\n+\tCommit_2   if (z == 0)\n+\tCommit_1   {\n+\tCommit_2     return x;\n+\tCommit_1   }\n+\tCommit_2   return y;\n+\tCommit_1 }\n+\tEOF\n+\n+\n+\tgit blame file.c | \\\n+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n+\t\tsed -e \"s/ *$//g\" > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors --diff-algorithm option' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit blame file.c --diff-algorithm histogram | \\\n+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n+\t\tsed -e \"s/ *$//g\" > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors diff.algorithm config variable' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit config diff.algorithm histogram &&\n+\tgit blame file.c | \\\n+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n+\t\tsed -e \"s/ *$//g\" > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame gives priority to --diff-algorithm over diff.algorithm' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit config diff.algorithm myers &&\n+\tgit blame file.c --diff-algorithm histogram | \\\n+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n+\t\tsed -e \"s/ *$//g\" > actual &&\n+\ttest_cmp expected actual\n+'\n+test_expect_success 'blame honors --minimal option' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_2 A\n+\tCommit_2 B\n+\tCommit_2 C\n+\tCommit_2 D\n+\tCommit_1 x\n+\tCommit_2 E\n+\tCommit_2 F\n+\tCommit_2 G\n+\tEOF\n+\n+\tgit blame file.txt --minimal | \\\n+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame respects the order of diff options' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_2 A\n+\tCommit_2 B\n+\tCommit_2 C\n+\tCommit_2 D\n+\tCommit_2 x\n+\tCommit_2 E\n+\tCommit_2 F\n+\tCommit_2 G\n+\tEOF\n+\n+\tgit blame file.txt --minimal --diff-algorithm myers | \\\n+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+test_done\n\nbase-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\n-- \ngitgitgadget\n"},{"id":"529859","messageId":"fde3dae1-bb11-45e8-9211-50ae003ca497@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v3.git.git.1761686060477.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] blame: make diff algorithm configurable","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-10-29T10:16:27Z","receivedAt":"2025-10-29T10:16:33Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Antonin\n\nOn 28/10/2025 21:14, Antonin Delpeuch via GitGitGadget wrote:\n> From: Antonin Delpeuch <antonin@delpeuch.eu>\n> \n> The diff algorithm used in 'git-blame(1)' is set to 'myers',\n> without the possibility to change it aside from the `--minimal` option.\n> \n> There has been long-standing interest in changing the default diff\n> algorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\n> for taking some steps towards that:\n> \n> https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n> \n> As a preparation for this move, it is worth making sure that the diff\n> algorithm is configurable where useful.\n> \n> Make it configurable in the `git-blame(1)` command by introducing the\n> `--diff-algorithm` option and make honor the `diff.algorithm` config\n> variable. Keep Myers diff as the default.\n> \n> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n> ---\n\nApart from a problem with clearing XDF_NEED_MINIMAL (which is really the \nfault of a terrible api) this is looking good.\n\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n>   \t\t}\n>   \t}\n>   \n> +\tif (!strcmp(var, \"diff.algorithm\")) {\n> +\t\tlong diff_algorithm;\n> +\t\tif (!value)\n> +\t\t\treturn config_error_nonbool(var);\n> +\t\tdiff_algorithm = parse_algorithm_value(value);\n> +\t\tif (diff_algorithm < 0)\n> +\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n> +\t\t\t\t     var, value);\n> +\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n\nUnfortunately XDF_DIFF_ALGORITHM_MASK does not include XDF_NEED_MINIMAL \nso if the user has a config file that looks like\n\t\n\t[diff]\n\t\talgorithm = minimal\n\t\talgorithm = myers\n\nWe'll parse it as \"minimal\" rather than \"myers\"\n\nAs we need to reset the diff algorithm in a number of places I think it \nwould be best to define a macro\n\n     \t#define CLEAR_DIFF_ALGORITHM(flags) \\\n\t\tflags &= ~(XDF_DIFF_ALGORITHM_MASK | XDF_NEED_MINIMAL)\n\nand use that where we want to reset the algorithm.\n> +\t\txdl_opts |= diff_algorithm;\n> +\t\treturn 0;\n> +\t}\n> +\n>   \tif (git_diff_heuristic_config(var, value, cb) < 0)\n>   \t\treturn -1;\n>   \tif (userdiff_config(var, value) < 0)\n> @@ -824,6 +837,38 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n>   \treturn 0;\n>   }\n>   \n> +static int blame_diff_algorithm_minimal(const struct option *option,\n> +\t\t\t\t\tconst char *arg, int unset)\n> +{\n> +\tint *opt = option->value;\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n\nThis is a change in behavior as we currently accept \"--no-minimal\" which \nclears XDF_NEED_MINIMAL\n> +\tBUG_ON_OPT_ARG(arg);\n> +\n> +\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n\nThis is correct becase we're about to set XDF_NEED_MINIMAL so it does \nnot matter that we leave it set here, but would still be clearer if it \nused the new macro I suggested above.\n\n> +\t*opt |= XDF_NEED_MINIMAL;> +\treturn 0;\n> +}\n> +\n> +static int blame_diff_algorithm_callback(const struct option *option,\n> +\t\t\t\t\t const char *arg, int unset)\n> +{\n> +\tint *opt = option->value;\n> +\tlong value = parse_algorithm_value(arg);\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\n> +\tif (value < 0)\n> +\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n> +\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n> +\n> +\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n\nThis is correct\n\n> +\t*opt |= value;\n> +\n> +\treturn 0;\n> +}\n> +\n\n> -\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n> +\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n> +\t\t\t       N_(\"spend extra cycles to find better match\"),\n\nThis is just copying the existing text so it is not a new problem but I \nthink it would be better if we said \"find a better\" rather than \"find \nbetter\". We should prehaps think about hiding this option now that we \nsupport --diff-algorithm.\n\n> +\t\t\t       PARSE_OPT_NONEG | PARSE_OPT_NOARG,\n\nAs I said above using PARSE_OPT_NONEG here is a regression\n\n> +\t\t\t       blame_diff_algorithm_minimal),\n> diff --git a/t/t8015-blame-diff-algorithm.sh b/t/t8015-blame-diff-algorithm.sh\n> new file mode 100755\n> index 0000000000..efc4b47ce1\n> --- /dev/null\n> +++ b/t/t8015-blame-diff-algorithm.sh\n> [...]\n> +test_expect_success 'blame uses Myers diff algorithm by default for now' '\n\nI'm not sure we need to say \"for now\" here.\n\n> +\tcat >expected <<-\\EOF &&\n> +\tCommit_2 int g(size_t u)\n> +\tCommit_1 {\n> +\tCommit_2   while (u < 30)\n> +\tCommit_1   {\n> +\tCommit_2     u++;\n> +\tCommit_1   }\n> +\tCommit_2   return u;\n> +\tCommit_1 }\n> +\tCommit_1\n> +\tCommit_2 int h(int x, int y, int z)\n> +\tCommit_1 {\n> +\tCommit_2   if (z == 0)\n> +\tCommit_1   {\n> +\tCommit_2     return x;\n> +\tCommit_1   }\n> +\tCommit_2   return y;\n> +\tCommit_1 }\n> +\tEOF\n> +\n> +\n\nThere's an extra blank line here\n\n> +\tgit blame file.c | \\\n\nWe don't pipe the output git commands as it hides unexpected failures. \nInstead you should redirect the output of git to a file and then process \nthat file with sed.\n\n> +\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n> +\t\tsed -e \"s/ *$//g\" > actual &&\n\nThis can be a single process by passing -e twice. It does not really \nmatter but neither pattern needs a trailing \"g\" as they only match once \nwithin the line.\n\n> +test_expect_success 'blame gives priority to --diff-algorithm over diff.algorithm' '\n> +\tcat >expected <<-\\EOF &&\n> +\tCommit_1 int g(size_t u)\n> +\tCommit_1 {\n> +\tCommit_1   while (u < 30)\n> +\tCommit_1   {\n> +\tCommit_1     u++;\n> +\tCommit_1   }\n> +\tCommit_1   return u;\n> +\tCommit_1 }\n> +\tCommit_2\n> +\tCommit_2 int h(int x, int y, int z)\n> +\tCommit_2 {\n> +\tCommit_2   if (z == 0)\n> +\tCommit_2   {\n> +\tCommit_2     return x;\n> +\tCommit_2   }\n> +\tCommit_2   return y;\n> +\tCommit_2 }\n> +\tEOF\n> +\n> +\tgit config diff.algorithm myers &&\n\nYou can use test_config() here which will clear the config setting at \nthe end of the test. Alternatively you can save a couple of processes by \nusing \"git -c diff.algorithm=myers blame ...\". This is setting the \nconfig to the default value, I wonder if it would be better to do\n\n\tgit -c diff.algorithm=histogram blame --diff-algorithm=myers\n\ninstead.\n\nThe coverage looks good\n\nThanks\n\nPhillip\n\n"},{"id":"529884","messageId":"xmqqms598s12.fsf@gitster.g","threadId":"64356","inReplyTo":"fde3dae1-bb11-45e8-9211-50ae003ca497@gmail.com","subject":"Re: [PATCH v3] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-29T18:46:49Z","receivedAt":"2025-10-29T18:46:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n>> +static int blame_diff_algorithm_minimal(const struct option *option,\n>> +\t\t\t\t\tconst char *arg, int unset)\n>> +{\n>> +\tint *opt = option->value;\n>> +\n>> +\tBUG_ON_OPT_NEG(unset);\n>\n> This is a change in behavior as we currently accept \"--no-minimal\" which \n> clears XDF_NEED_MINIMAL\n\nAh, I missed this; thanks for a careful reading.\n\n> As I said above using PARSE_OPT_NONEG here is a regression\n>\n>> +\t\t\t       blame_diff_algorithm_minimal),\n>> diff --git a/t/t8015-blame-diff-algorithm.sh b/t/t8015-blame-diff-algorithm.sh\n>> new file mode 100755\n>> index 0000000000..efc4b47ce1\n>> --- /dev/null\n>> +++ b/t/t8015-blame-diff-algorithm.sh\n>> [...]\n>> +test_expect_success 'blame uses Myers diff algorithm by default for now' '\n>\n> I'm not sure we need to say \"for now\" here.\n\nWe shouldn't.\n"},{"id":"529931","messageId":"33d44dc6-36b3-4736-b3ed-96861a3c4003@delpeuch.eu","threadId":"64356","inReplyTo":"fde3dae1-bb11-45e8-9211-50ae003ca497@gmail.com","subject":"Re: [PATCH v3] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch","fromEmail":"antonin@delpeuch.eu","sentAt":"2025-10-30T09:22:41Z","receivedAt":"2025-10-30T09:35:49Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"Hi Phillip,\n\nOn 29/10/2025 11:16, Phillip Wood wrote:\n> Unfortunately XDF_DIFF_ALGORITHM_MASK does not include \n> XDF_NEED_MINIMAL so if the user has a config file that looks like\n>\n>     [diff]\n>         algorithm = minimal\n>         algorithm = myers\n>\n> We'll parse it as \"minimal\" rather than \"myers\"\n>\n> As we need to reset the diff algorithm in a number of places I think \n> it would be best to define a macro\n>\n>         #define CLEAR_DIFF_ALGORITHM(flags) \\\n>         flags &= ~(XDF_DIFF_ALGORITHM_MASK | XDF_NEED_MINIMAL)\n\nOuch, good catch! This problem is affecting other places as well.\n\nI'm wondering if we couldn't even add XDF_NEED_MINIMAL to \nXDF_DIFF_ALGORITHM_MASK. I've reviewed all the places where \nXDF_DIFF_ALGORITHM_MASK is used, and it seems that in all cases it would \neither preserve the existing behaviour (potentially allowing us to \nremove an accompanying \"DIFF_XDL_CLR(opts, NEED_MINIMAL);\" macro which \nbecomes redundant), or in some other cases it would fix a similar issue \n(for instance, in merge-file.c).\n\nIs your suggestion to introduce a new macro motivated by stability \nconcerns? I'm aware that xdiff is used in other code bases as a library, \nso I guess changing XDF_DIFF_ALGORITHM_MASK can indeed be seen as a \nbreaking change.\n\nAntonin\n\n"},{"id":"529950","messageId":"fa9c953b-a03f-44ab-962e-3eb4ee335b5b@gmail.com","threadId":"64356","inReplyTo":"33d44dc6-36b3-4736-b3ed-96861a3c4003@delpeuch.eu","subject":"Re: [PATCH v3] blame: make diff algorithm configurable","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-10-30T10:47:43Z","receivedAt":"2025-10-30T10:47:47Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Antonin\n\nOn 30/10/2025 09:22, Antonin Delpeuch wrote:\n> On 29/10/2025 11:16, Phillip Wood wrote:\n>> Unfortunately XDF_DIFF_ALGORITHM_MASK does not include \n>> XDF_NEED_MINIMAL so if the user has a config file that looks like\n>>\n>>     [diff]\n>>         algorithm = minimal\n>>         algorithm = myers\n>>\n>> We'll parse it as \"minimal\" rather than \"myers\"\n>>\n>> As we need to reset the diff algorithm in a number of places I think \n>> it would be best to define a macro\n>>\n>>         #define CLEAR_DIFF_ALGORITHM(flags) \\\n>>         flags &= ~(XDF_DIFF_ALGORITHM_MASK | XDF_NEED_MINIMAL)\n> \n> Ouch, good catch! This problem is affecting other places as well.\n> \n> I'm wondering if we couldn't even add XDF_NEED_MINIMAL to \n> XDF_DIFF_ALGORITHM_MASK. I've reviewed all the places where \n> XDF_DIFF_ALGORITHM_MASK is used, and it seems that in all cases it would \n> either preserve the existing behaviour (potentially allowing us to \n> remove an accompanying \"DIFF_XDL_CLR(opts, NEED_MINIMAL);\" macro which \n> becomes redundant), or in some other cases it would fix a similar issue \n> (for instance, in merge-file.c).\n\nThanks for taking the time to look at the other uses of \nXDF_DIFF_ALGORITHM_MASK. I think adding XDF_NEED_MINIMAL to \nXDF_DIFF_ALGORITHM_MASK is a good idea as I think we always treat \n\"minimal\" as another diff algorithm, rather than a variant of \"myers\" \nwhich is how it is implemented in xdiff.\n\n> Is your suggestion to introduce a new macro motivated by stability \n> concerns? I'm aware that xdiff is used in other code bases as a library, \n> so I guess changing XDF_DIFF_ALGORITHM_MASK can indeed be seen as a \n> breaking change.\n\nWhile it is a breaking change, tweaking XDF_DIFF_ALGORITHM_MASK seems \nlike a better idea to me. I wondered about it yesterday but worried it \nwould end up being too much work to review all the existing uses.\n\nThanks\n\nPhillip\n\n> Antonin\n> \n\n"},{"id":"530067","messageId":"pull.2075.v4.git.git.1762034252.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v3.git.git.1761686060477.gitgitgadget@gmail.com","subject":"[PATCH v4 0/2] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-01T21:57:30Z","receivedAt":"2025-11-01T21:57:37Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"Changes since v3:\n\n * fix resetting of diff algorithm by adding XDF_NEED_MINIMAL to\n   XDF_DIFF_ALGORITHM_MASK\n * restore --no-minimal support\n * fix typo in description of --minimal option\n * remove the 'for now' in test description\n * remove piping in tests\n * pass configuration variables to the blame command directly instead of\n   calling 'git config'\n\nAntonin Delpeuch (2):\n  xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n  blame: make diff algorithm configurable\n\n Documentation/diff-algorithm-option.adoc |  20 +++\n Documentation/diff-options.adoc          |  21 +--\n Documentation/git-blame.adoc             |   2 +\n builtin/blame.c                          |  52 +++++-\n diff.c                                   |   2 -\n merge-ort.c                              |   2 -\n t/meson.build                            |   1 +\n t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n xdiff/xdiff.h                            |   2 +-\n 9 files changed, 279 insertions(+), 26 deletions(-)\n create mode 100644 Documentation/diff-algorithm-option.adoc\n create mode 100755 t/t8015-blame-diff-algorithm.sh\n\n\nbase-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v4\nPull-Request: https://github.com/git/git/pull/2075\n\nRange-diff vs v3:\n\n -:  ---------- > 1:  e81a5d2bd2 xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n 1:  b8bdb03516 ! 2:  920a6f3acb blame: make diff algorithm configurable\n     @@ builtin/blame.c: static int blame_move_callback(const struct option *option, con\n      +{\n      +\tint *opt = option->value;\n      +\n     -+\tBUG_ON_OPT_NEG(unset);\n      +\tBUG_ON_OPT_ARG(arg);\n      +\n      +\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n     -+\t*opt |= XDF_NEED_MINIMAL;\n     ++\tif (!unset)\n     ++\t\t*opt |= XDF_NEED_MINIMAL;\n      +\n      +\treturn 0;\n      +}\n     @@ builtin/blame.c: int cmd_blame(int argc,\n       \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n      -\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n      +\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n     -+\t\t\t       N_(\"spend extra cycles to find better match\"),\n     -+\t\t\t       PARSE_OPT_NONEG | PARSE_OPT_NOARG,\n     -+\t\t\t       blame_diff_algorithm_minimal),\n     ++\t\t\t       N_(\"spend extra cycles to find a better match\"),\n     ++\t\t\t       PARSE_OPT_NOARG, blame_diff_algorithm_minimal),\n       \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n       \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n       \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tGIT_AUTHOR_NAME=Commit_2 git commit -m Commit_2\n      +'\n      +\n     -+test_expect_success 'blame uses Myers diff algorithm by default for now' '\n     ++test_expect_success 'blame uses Myers diff algorithm by default' '\n      +\tcat >expected <<-\\EOF &&\n      +\tCommit_2 int g(size_t u)\n      +\tCommit_1 {\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_1 }\n      +\tEOF\n      +\n     -+\n     -+\tgit blame file.c | \\\n     -+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n     -+\t\tsed -e \"s/ *$//g\" > actual &&\n     ++\tgit blame file.c > output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n     ++\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 }\n      +\tEOF\n      +\n     -+\tgit blame file.c --diff-algorithm histogram | \\\n     -+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n     -+\t\tsed -e \"s/ *$//g\" > actual &&\n     ++\tgit blame file.c --diff-algorithm histogram > output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n     ++\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 }\n      +\tEOF\n      +\n     -+\tgit config diff.algorithm histogram &&\n     -+\tgit blame file.c | \\\n     -+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n     -+\t\tsed -e \"s/ *$//g\" > actual &&\n     ++\tgit -c diff.algorithm=histogram blame file.c > output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n     ++\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 }\n      +\tEOF\n      +\n     -+\tgit config diff.algorithm myers &&\n     -+\tgit blame file.c --diff-algorithm histogram | \\\n     -+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" | \\\n     -+\t\tsed -e \"s/ *$//g\" > actual &&\n     ++\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n     ++\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n      +\ttest_cmp expected actual\n      +'\n     ++\n      +test_expect_success 'blame honors --minimal option' '\n      +\tcat >expected <<-\\EOF &&\n      +\tCommit_1 x\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 G\n      +\tEOF\n      +\n     -+\tgit blame file.txt --minimal | \\\n     -+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" > actual &&\n     ++\tgit blame file.txt --minimal > output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 G\n      +\tEOF\n      +\n     -+\tgit blame file.txt --minimal --diff-algorithm myers | \\\n     -+\t\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" > actual &&\n     ++\tgit blame file.txt --minimal --diff-algorithm myers > output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     -+\n      +test_done\n\n-- \ngitgitgadget\n"},{"id":"530068","messageId":"e81a5d2bd23add19e04184f6b37910bc89a514a5.1762034252.git.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v4.git.git.1762034252.gitgitgadget@gmail.com","subject":"[PATCH v4 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-01T21:57:31Z","receivedAt":"2025-11-01T21:57:39Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience\nand histogram diffs, not for the minimal one. This means that when\nreseting the diff algorithm to the default one, one needs to separately\nclear the bit for the minimal diff. There are places in the code that fail\nto do that: merge-ort.c and builtin/merge-file.c.\n\nAdd the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate\nclearing of this bit in the places where it hasn't been forgotten.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n diff.c        | 2 --\n merge-ort.c   | 2 --\n xdiff/xdiff.h | 2 +-\n 3 files changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 87fa16b730..6ce3591c5b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3526,8 +3526,6 @@ static int set_diff_algorithm(struct diff_options *opts,\n \tif (value < 0)\n \t\treturn -1;\n \n-\t/* clear out previous settings */\n-\tDIFF_XDL_CLR(opts, NEED_MINIMAL);\n \topts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n \topts->xdl_opts |= value;\n \ndiff --git a/merge-ort.c b/merge-ort.c\nindex 29858074f9..9b2b0fce7e 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -5495,8 +5495,6 @@ int parse_merge_opt(struct merge_options *opt, const char *s)\n \t\tlong value = parse_algorithm_value(arg);\n \t\tif (value < 0)\n \t\t\treturn -1;\n-\t\t/* clear out previous settings */\n-\t\tDIFF_XDL_CLR(opt, NEED_MINIMAL);\n \t\topt->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n \t\topt->xdl_opts |= value;\n \t}\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 2cecde5afe..dc370712e9 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -43,7 +43,7 @@ extern \"C\" {\n \n #define XDF_PATIENCE_DIFF (1 << 14)\n #define XDF_HISTOGRAM_DIFF (1 << 15)\n-#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)\n+#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF | XDF_NEED_MINIMAL)\n #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)\n \n #define XDF_INDENT_HEURISTIC (1 << 23)\n-- \ngitgitgadget\n\n"},{"id":"530069","messageId":"920a6f3acbc86e72c6ea236f8dbd3d559398409a.1762034252.git.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v4.git.git.1762034252.gitgitgadget@gmail.com","subject":"[PATCH v4 2/2] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-01T21:57:32Z","receivedAt":"2025-11-01T21:57:42Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe diff algorithm used in 'git-blame(1)' is set to 'myers',\nwithout the possibility to change it aside from the `--minimal` option.\n\nThere has been long-standing interest in changing the default diff\nalgorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\nfor taking some steps towards that:\n\nhttps://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n\nAs a preparation for this move, it is worth making sure that the diff\nalgorithm is configurable where useful.\n\nMake it configurable in the `git-blame(1)` command by introducing the\n`--diff-algorithm` option and make honor the `diff.algorithm` config\nvariable. Keep Myers diff as the default.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n Documentation/diff-algorithm-option.adoc |  20 +++\n Documentation/diff-options.adoc          |  21 +--\n Documentation/git-blame.adoc             |   2 +\n builtin/blame.c                          |  52 +++++-\n t/meson.build                            |   1 +\n t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n 6 files changed, 278 insertions(+), 21 deletions(-)\n create mode 100644 Documentation/diff-algorithm-option.adoc\n create mode 100755 t/t8015-blame-diff-algorithm.sh\n\ndiff --git a/Documentation/diff-algorithm-option.adoc b/Documentation/diff-algorithm-option.adoc\nnew file mode 100644\nindex 0000000000..8e3a0b63d7\n--- /dev/null\n+++ b/Documentation/diff-algorithm-option.adoc\n@@ -0,0 +1,20 @@\n+`--diff-algorithm=(patience|minimal|histogram|myers)`::\n+\tChoose a diff algorithm. The variants are as follows:\n++\n+--\n+   `default`;;\n+   `myers`;;\n+\tThe basic greedy diff algorithm. Currently, this is the default.\n+   `minimal`;;\n+\tSpend extra time to make sure the smallest possible diff is\n+\tproduced.\n+   `patience`;;\n+\tUse \"patience diff\" algorithm when generating patches.\n+   `histogram`;;\n+\tThis algorithm extends the patience algorithm to \"support\n+\tlow-occurrence common elements\".\n+--\n++\n+For instance, if you configured the `diff.algorithm` variable to a\n+non-default value and want to use the default one, then you\n+have to use `--diff-algorithm=default` option.\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex ae31520f7f..9cdad6f72a 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -197,26 +197,7 @@ and starts with _<text>_, this algorithm attempts to prevent it from\n appearing as a deletion or addition in the output. It uses the \"patience\n diff\" algorithm internally.\n \n-`--diff-algorithm=(patience|minimal|histogram|myers)`::\n-\tChoose a diff algorithm. The variants are as follows:\n-+\n---\n-   `default`;;\n-   `myers`;;\n-\tThe basic greedy diff algorithm. Currently, this is the default.\n-   `minimal`;;\n-\tSpend extra time to make sure the smallest possible diff is\n-\tproduced.\n-   `patience`;;\n-\tUse \"patience diff\" algorithm when generating patches.\n-   `histogram`;;\n-\tThis algorithm extends the patience algorithm to \"support\n-\tlow-occurrence common elements\".\n---\n-+\n-For instance, if you configured the `diff.algorithm` variable to a\n-non-default value and want to use the default one, then you\n-have to use `--diff-algorithm=default` option.\n+include::diff-algorithm-option.adoc[]\n \n `--stat[=<width>[,<name-width>[,<count>]]]`::\n \tGenerate a diffstat. By default, as much space as necessary\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex e438d28625..adcbb6f5dc 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -85,6 +85,8 @@ include::blame-options.adoc[]\n \tIgnore whitespace when comparing the parent's version and\n \tthe child's to find where the lines came from.\n \n+include::diff-algorithm-option.adoc[]\n+\n --abbrev=<n>::\n \tInstead of using the default 7+1 hexadecimal digits as the\n \tabbreviated object name, use <m>+1 digits, where <m> is at\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 2703820258..888ce708a6 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n \t\t}\n \t}\n \n+\tif (!strcmp(var, \"diff.algorithm\")) {\n+\t\tlong diff_algorithm;\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tdiff_algorithm = parse_algorithm_value(value);\n+\t\tif (diff_algorithm < 0)\n+\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n+\t\t\t\t     var, value);\n+\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t\txdl_opts |= diff_algorithm;\n+\t\treturn 0;\n+\t}\n+\n \tif (git_diff_heuristic_config(var, value, cb) < 0)\n \t\treturn -1;\n \tif (userdiff_config(var, value) < 0)\n@@ -824,6 +837,38 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n \treturn 0;\n }\n \n+static int blame_diff_algorithm_minimal(const struct option *option,\n+\t\t\t\t\tconst char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\n+\tBUG_ON_OPT_ARG(arg);\n+\n+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n+\tif (!unset)\n+\t\t*opt |= XDF_NEED_MINIMAL;\n+\n+\treturn 0;\n+}\n+\n+static int blame_diff_algorithm_callback(const struct option *option,\n+\t\t\t\t\t const char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\tlong value = parse_algorithm_value(arg);\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tif (value < 0)\n+\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n+\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n+\n+\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n+\t*opt |= value;\n+\n+\treturn 0;\n+}\n+\n static int is_a_rev(const char *name)\n {\n \tstruct object_id oid;\n@@ -915,11 +960,16 @@ int cmd_blame(int argc,\n \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n+\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n+\t\t\t       N_(\"choose a diff algorithm\"),\n+\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n-\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n+\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n+\t\t\t       N_(\"spend extra cycles to find a better match\"),\n+\t\t\t       PARSE_OPT_NOARG, blame_diff_algorithm_minimal),\n \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\ndiff --git a/t/meson.build b/t/meson.build\nindex 401b24e50e..9f2fe7af8b 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -955,6 +955,7 @@ integration_tests = [\n   't8012-blame-colors.sh',\n   't8013-blame-ignore-revs.sh',\n   't8014-blame-ignore-fuzzy.sh',\n+  't8015-blame-diff-algorithm.sh',\n   't8020-last-modified.sh',\n   't9001-send-email.sh',\n   't9002-column.sh',\ndiff --git a/t/t8015-blame-diff-algorithm.sh b/t/t8015-blame-diff-algorithm.sh\nnew file mode 100755\nindex 0000000000..5318e18cb3\n--- /dev/null\n+++ b/t/t8015-blame-diff-algorithm.sh\n@@ -0,0 +1,203 @@\n+#!/bin/sh\n+\n+test_description='git blame with specific diff algorithm'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tcat >file.c <<-\\EOF &&\n+\tint f(int x, int y)\n+\t{\n+\t  if (x == 0)\n+\t  {\n+\t    return y;\n+\t  }\n+\t  return x;\n+\t}\n+\n+\tint g(size_t u)\n+\t{\n+\t  while (u < 30)\n+\t  {\n+\t    u++;\n+\t  }\n+\t  return u;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x x >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Commit_1 git commit -m Commit_1 &&\n+\n+\tcat >file.c <<-\\EOF &&\n+\tint g(size_t u)\n+\t{\n+\t  while (u < 30)\n+\t  {\n+\t    u++;\n+\t  }\n+\t  return u;\n+\t}\n+\n+\tint h(int x, int y, int z)\n+\t{\n+\t  if (z == 0)\n+\t  {\n+\t    return x;\n+\t  }\n+\t  return y;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x A B C D x E F G >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Commit_2 git commit -m Commit_2\n+'\n+\n+test_expect_success 'blame uses Myers diff algorithm by default' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_2 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_2   while (u < 30)\n+\tCommit_1   {\n+\tCommit_2     u++;\n+\tCommit_1   }\n+\tCommit_2   return u;\n+\tCommit_1 }\n+\tCommit_1\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_1 {\n+\tCommit_2   if (z == 0)\n+\tCommit_1   {\n+\tCommit_2     return x;\n+\tCommit_1   }\n+\tCommit_2   return y;\n+\tCommit_1 }\n+\tEOF\n+\n+\tgit blame file.c > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors --diff-algorithm option' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit blame file.c --diff-algorithm histogram > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors diff.algorithm config variable' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit -c diff.algorithm=histogram blame file.c > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame gives priority to --diff-algorithm over diff.algorithm' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors --minimal option' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_2 A\n+\tCommit_2 B\n+\tCommit_2 C\n+\tCommit_2 D\n+\tCommit_1 x\n+\tCommit_2 E\n+\tCommit_2 F\n+\tCommit_2 G\n+\tEOF\n+\n+\tgit blame file.txt --minimal > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame respects the order of diff options' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_2 A\n+\tCommit_2 B\n+\tCommit_2 C\n+\tCommit_2 D\n+\tCommit_2 x\n+\tCommit_2 E\n+\tCommit_2 F\n+\tCommit_2 G\n+\tEOF\n+\n+\tgit blame file.txt --minimal --diff-algorithm myers > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n"},{"id":"530122","messageId":"6ec34cb1-4149-48b3-8c15-fe3460aae729@gmail.com","threadId":"64356","inReplyTo":"e81a5d2bd23add19e04184f6b37910bc89a514a5.1762034252.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-03T14:32:33Z","receivedAt":"2025-11-03T14:32:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Antonin\n\nOn 01/11/2025 21:57, Antonin Delpeuch via GitGitGadget wrote:\n> From: Antonin Delpeuch <antonin@delpeuch.eu>\n> \n> The XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience\n> and histogram diffs, not for the minimal one. This means that when\n> reseting the diff algorithm to the default one, one needs to separately\n> clear the bit for the minimal diff. There are places in the code that fail\n> to do that: merge-ort.c and builtin/merge-file.c.\n> \n> Add the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate\n> clearing of this bit in the places where it hasn't been forgotten.\n\nNicely explained. This is a useful improvement that should prevent \nerrors in the future. After this patch there are no users of \nDIFF_XDL_CLR() so we should probably remove that macro. I'm not sure it \nmakes sense to remove the comments that have been deleted below as we're \nstill clearing the old setting. Apart from that this all looks good.\n\nThanks\n\nPhillip\n\n> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n> ---\n>   diff.c        | 2 --\n>   merge-ort.c   | 2 --\n>   xdiff/xdiff.h | 2 +-\n>   3 files changed, 1 insertion(+), 5 deletions(-)\n> \n> diff --git a/diff.c b/diff.c\n> index 87fa16b730..6ce3591c5b 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3526,8 +3526,6 @@ static int set_diff_algorithm(struct diff_options *opts,\n>   \tif (value < 0)\n>   \t\treturn -1;\n>   \n> -\t/* clear out previous settings */\n> -\tDIFF_XDL_CLR(opts, NEED_MINIMAL);\n>   \topts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n>   \topts->xdl_opts |= value;\n>   \n> diff --git a/merge-ort.c b/merge-ort.c\n> index 29858074f9..9b2b0fce7e 100644\n> --- a/merge-ort.c\n> +++ b/merge-ort.c\n> @@ -5495,8 +5495,6 @@ int parse_merge_opt(struct merge_options *opt, const char *s)\n>   \t\tlong value = parse_algorithm_value(arg);\n>   \t\tif (value < 0)\n>   \t\t\treturn -1;\n> -\t\t/* clear out previous settings */\n> -\t\tDIFF_XDL_CLR(opt, NEED_MINIMAL);\n>   \t\topt->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n>   \t\topt->xdl_opts |= value;\n>   \t}\n> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\n> index 2cecde5afe..dc370712e9 100644\n> --- a/xdiff/xdiff.h\n> +++ b/xdiff/xdiff.h\n> @@ -43,7 +43,7 @@ extern \"C\" {\n>   \n>   #define XDF_PATIENCE_DIFF (1 << 14)\n>   #define XDF_HISTOGRAM_DIFF (1 << 15)\n> -#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)\n> +#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF | XDF_NEED_MINIMAL)\n>   #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)\n>   \n>   #define XDF_INDENT_HEURISTIC (1 << 23)\n\n"},{"id":"530123","messageId":"d0bee2f2-106c-42cf-8101-c76bb54ee1ba@gmail.com","threadId":"64356","inReplyTo":"920a6f3acbc86e72c6ea236f8dbd3d559398409a.1762034252.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 2/2] blame: make diff algorithm configurable","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-03T14:32:39Z","receivedAt":"2025-11-03T14:32:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Antonin\n\nThanks for re-rolling, this is looking pretty sound, I've left a couple \nof fairly minor comments below.\n\nOn 01/11/2025 21:57, Antonin Delpeuch via GitGitGadget wrote:\n> +static int blame_diff_algorithm_minimal(const struct option *option,\n> +\t\t\t\t\tconst char *arg, int unset)\n> +{\n> +\tint *opt = option->value;\n> +\n> +\tBUG_ON_OPT_ARG(arg);\n> +\n> +\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n> +\tif (!unset)\n> +\t\t*opt |= XDF_NEED_MINIMAL;\nOne thing I'd not thought about before was the interaction between \n\"--no-minimal\" and \"--diff-algorithm\" The code above makes \n\"--no-minimal\" behave like \"diff-algorithm=myers\" which is consistent \nwith the current behavior where the only options for the diff algorithm \nare \"minimal\" or \"myers\". An alternative would be for \"--no-minimal\" to \njust clear XDF_NEED_MINIMAL and behave like a no-op if it is given after \n\"--diff-algorithm=patience\" or \"--diff-algorithm=histogram\". I don't \nreally have a strong preference either way.\n\n> +static int blame_diff_algorithm_callback(const struct option *option,\n> +\t\t\t\t\t const char *arg, int unset)\n> +{\n> +\tint *opt = option->value;\n> +\tlong value = parse_algorithm_value(arg);\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\n> +\tif (value < 0)\n> +\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n> +\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n> +\n> +\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n\nWe can just use XDF_DIFF_ALGORITHM_MASK now that we've added \nXDF_NEED_MINMAL to it in the last commit.\n\n> @@ -915,11 +960,16 @@ int cmd_blame(int argc,\n>   \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n>   \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n>   \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n> +\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n> +\t\t\t       N_(\"choose a diff algorithm\"),\n> +\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n>   \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n>   \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n>   \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n>   \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n> -\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n> +\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n> +\t\t\t       N_(\"spend extra cycles to find a better match\"),\n> +\t\t\t       PARSE_OPT_NOARG, blame_diff_algorithm_minimal),\n\nGiven the potential for confusing interactions between \"--no-minimal\" \nand \"--diff-algorithm\" I think it would be worth adding OPT_HIDDEN here.\n\n> diff --git a/t/t8015-blame-diff-algorithm.sh b/t/t8015-blame-diff-algorithm.sh\n> new file mode 100755\n> index 0000000000..5318e18cb3\n> --- /dev/null\n> +++ b/t/t8015-blame-diff-algorithm.sh\n> @@ -0,0 +1,203 @@\n> + [...]\n> +\tgit blame file.c > output &&\n> +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n> +\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n\nThis would be more efficient if it was written as\n\n\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n\t    -e \"s/ *$//g\" output >actual\n\nOur test suite is really slow on windows so it is worth trying to avoid \ncreating unnecessary processes.\n\nThanks\n\nPhillip\n\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success 'blame honors --diff-algorithm option' '\n> +\tcat >expected <<-\\EOF &&\n> +\tCommit_1 int g(size_t u)\n> +\tCommit_1 {\n> +\tCommit_1   while (u < 30)\n> +\tCommit_1   {\n> +\tCommit_1     u++;\n> +\tCommit_1   }\n> +\tCommit_1   return u;\n> +\tCommit_1 }\n> +\tCommit_2\n> +\tCommit_2 int h(int x, int y, int z)\n> +\tCommit_2 {\n> +\tCommit_2   if (z == 0)\n> +\tCommit_2   {\n> +\tCommit_2     return x;\n> +\tCommit_2   }\n> +\tCommit_2   return y;\n> +\tCommit_2 }\n> +\tEOF\n> +\n> +\tgit blame file.c --diff-algorithm histogram > output &&\n> +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n> +\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success 'blame honors diff.algorithm config variable' '\n> +\tcat >expected <<-\\EOF &&\n> +\tCommit_1 int g(size_t u)\n> +\tCommit_1 {\n> +\tCommit_1   while (u < 30)\n> +\tCommit_1   {\n> +\tCommit_1     u++;\n> +\tCommit_1   }\n> +\tCommit_1   return u;\n> +\tCommit_1 }\n> +\tCommit_2\n> +\tCommit_2 int h(int x, int y, int z)\n> +\tCommit_2 {\n> +\tCommit_2   if (z == 0)\n> +\tCommit_2   {\n> +\tCommit_2     return x;\n> +\tCommit_2   }\n> +\tCommit_2   return y;\n> +\tCommit_2 }\n> +\tEOF\n> +\n> +\tgit -c diff.algorithm=histogram blame file.c > output &&\n> +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n> +\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success 'blame gives priority to --diff-algorithm over diff.algorithm' '\n> +\tcat >expected <<-\\EOF &&\n> +\tCommit_1 int g(size_t u)\n> +\tCommit_1 {\n> +\tCommit_1   while (u < 30)\n> +\tCommit_1   {\n> +\tCommit_1     u++;\n> +\tCommit_1   }\n> +\tCommit_1   return u;\n> +\tCommit_1 }\n> +\tCommit_2\n> +\tCommit_2 int h(int x, int y, int z)\n> +\tCommit_2 {\n> +\tCommit_2   if (z == 0)\n> +\tCommit_2   {\n> +\tCommit_2     return x;\n> +\tCommit_2   }\n> +\tCommit_2   return y;\n> +\tCommit_2 }\n> +\tEOF\n> +\n> +\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram &&\n> +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n> +\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success 'blame honors --minimal option' '\n> +\tcat >expected <<-\\EOF &&\n> +\tCommit_1 x\n> +\tCommit_1 x\n> +\tCommit_1 x\n> +\tCommit_2 A\n> +\tCommit_2 B\n> +\tCommit_2 C\n> +\tCommit_2 D\n> +\tCommit_1 x\n> +\tCommit_2 E\n> +\tCommit_2 F\n> +\tCommit_2 G\n> +\tEOF\n> +\n> +\tgit blame file.txt --minimal > output &&\n> +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success 'blame respects the order of diff options' '\n> +\tcat >expected <<-\\EOF &&\n> +\tCommit_1 x\n> +\tCommit_1 x\n> +\tCommit_1 x\n> +\tCommit_2 A\n> +\tCommit_2 B\n> +\tCommit_2 C\n> +\tCommit_2 D\n> +\tCommit_2 x\n> +\tCommit_2 E\n> +\tCommit_2 F\n> +\tCommit_2 G\n> +\tEOF\n> +\n> +\tgit blame file.txt --minimal --diff-algorithm myers > output &&\n> +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_done\n\n"},{"id":"530129","messageId":"xmqqh5vbum6k.fsf@gitster.g","threadId":"64356","inReplyTo":"d0bee2f2-106c-42cf-8101-c76bb54ee1ba@gmail.com","subject":"Re: [PATCH v4 2/2] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-03T16:15:47Z","receivedAt":"2025-11-03T16:15:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> One thing I'd not thought about before was the interaction between \n> \"--no-minimal\" and \"--diff-algorithm\" The code above makes \n> \"--no-minimal\" behave like \"diff-algorithm=myers\" which is consistent \n> with the current behavior where the only options for the diff algorithm \n> are \"minimal\" or \"myers\". An alternative would be for \"--no-minimal\" to \n> just clear XDF_NEED_MINIMAL and behave like a no-op if it is given after \n> \"--diff-algorithm=patience\" or \"--diff-algorithm=histogram\". I don't \n> really have a strong preference either way.\n\nGood observation.\n\nIn the longer term, I think we would be better off if we treated\n\"minimal\" just like \"histogram\" and \"patience\", in that\n\n (1) If the command takes --diff-algorithm=<name>, giving it as the\n     <name> would override the previous setting.\n\n (2) If the command takes --<name> (i.e. \"git diff --histogram\"),\n     giving \"--no-<name>\" results in an error.\n\n (3) If the command takes --<name>, it should take all the variants\n     as <name>, not just selected few, or it shouldn't take any.\n\nIOW, we should deprecate \"blame --no-minimal\" as a past mistake, and\nin the longer term deprecate \"blame --minimal\" and tell users to use\n\"--diff-algorithm=minimal\" instead.\n\nIf Antonin's series wants to teach --histogram and --patience to\n\"git blame\", then we can and should keep \"blame --minimal\" (i.e.,\n(3) above), but in that case, \"blame --no-minimal\" should still go\n(i.e., (2) above).  Under the new world order where there are more\nthan the \"minimal/no-minimal?\"  binary choice, where you can specify\nother algorithms from the usual repertoire, the option \"no-minimal\"\nno longer makes sense.\n\n\n"},{"id":"530339","messageId":"xmqqo6pehpl6.fsf@gitster.g","threadId":"64356","inReplyTo":"xmqqh5vbum6k.fsf@gitster.g","subject":"Re: [PATCH v4 2/2] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-06T20:29:41Z","receivedAt":"2025-11-06T20:29:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n>> One thing I'd not thought about before was the interaction between \n>> \"--no-minimal\" and \"--diff-algorithm\" The code above makes \n>> \"--no-minimal\" behave like \"diff-algorithm=myers\" which is consistent \n>> with the current behavior where the only options for the diff algorithm \n>> are \"minimal\" or \"myers\". An alternative would be for \"--no-minimal\" to \n>> just clear XDF_NEED_MINIMAL and behave like a no-op if it is given after \n>> \"--diff-algorithm=patience\" or \"--diff-algorithm=histogram\". I don't \n>> really have a strong preference either way.\n>\n> Good observation.\n>\n> In the longer term, I think we would be better off if we treated\n> \"minimal\" just like \"histogram\" and \"patience\", in that\n> ...\n> other algorithms from the usual repertoire, the option \"no-minimal\"\n> no longer makes sense.\n\nJust to avoid confusion, the idea above to deprecate and kill\n\"--no-minimal\" is totally outside of this topic to teach \"git blame\"\nthe \"--diff-algorithm=<algo>\" command line option.\n\nI think in the shorter term I think it is OK for the implementation\nto do whatever it happens to do when given \"--no-minimal\", and as\nyou observed, it is fine to behave like \"--diff-algorithm=myers\".\nI also like your idea to hide --minimal with the OPT_HIDDEN bit.\n\nThanks.\n"},{"id":"530345","messageId":"pull.2075.v5.git.git.1762468914.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v4.git.git.1762034252.gitgitgadget@gmail.com","subject":"[PATCH v5 0/2] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-06T22:41:52Z","receivedAt":"2025-11-06T22:41:56Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"Changes since v4:\n\n * hide --minimal option\n * simplify tests to minimize spun processes\n * remove redundant XDF_NEED_MINIMAL in bit mask\n\nAntonin Delpeuch (2):\n  xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n  blame: make diff algorithm configurable\n\n Documentation/diff-algorithm-option.adoc |  20 +++\n Documentation/diff-options.adoc          |  21 +--\n Documentation/git-blame.adoc             |   2 +\n builtin/blame.c                          |  52 +++++-\n diff.c                                   |   2 -\n merge-ort.c                              |   2 -\n t/meson.build                            |   1 +\n t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n xdiff/xdiff.h                            |   2 +-\n 9 files changed, 279 insertions(+), 26 deletions(-)\n create mode 100644 Documentation/diff-algorithm-option.adoc\n create mode 100755 t/t8015-blame-diff-algorithm.sh\n\n\nbase-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v5\nPull-Request: https://github.com/git/git/pull/2075\n\nRange-diff vs v4:\n\n 1:  e81a5d2bd2 = 1:  e81a5d2bd2 xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n 2:  920a6f3acb ! 2:  60015bbada blame: make diff algorithm configurable\n     @@ builtin/blame.c: static int blame_move_callback(const struct option *option, con\n      +\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n      +\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n      +\n     -+\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n     ++\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n      +\t*opt |= value;\n      +\n      +\treturn 0;\n     @@ builtin/blame.c: int cmd_blame(int argc,\n      -\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n      +\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n      +\t\t\t       N_(\"spend extra cycles to find a better match\"),\n     -+\t\t\t       PARSE_OPT_NOARG, blame_diff_algorithm_minimal),\n     ++\t\t\t       PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, blame_diff_algorithm_minimal),\n       \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n       \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n       \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tEOF\n      +\n      +\tgit -c diff.algorithm=histogram blame file.c > output &&\n     -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n     -+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n     ++\t    -e \"s/ *$//g\" output > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 }\n      +\tEOF\n      +\n     -+\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram &&\n     -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n     -+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n     ++\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram > output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n     ++\t    -e \"s/ *$//g\" output > actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n\n-- \ngitgitgadget\n"},{"id":"530346","messageId":"e81a5d2bd23add19e04184f6b37910bc89a514a5.1762468914.git.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v5.git.git.1762468914.gitgitgadget@gmail.com","subject":"[PATCH v5 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-06T22:41:53Z","receivedAt":"2025-11-06T22:41:58Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience\nand histogram diffs, not for the minimal one. This means that when\nreseting the diff algorithm to the default one, one needs to separately\nclear the bit for the minimal diff. There are places in the code that fail\nto do that: merge-ort.c and builtin/merge-file.c.\n\nAdd the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate\nclearing of this bit in the places where it hasn't been forgotten.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n diff.c        | 2 --\n merge-ort.c   | 2 --\n xdiff/xdiff.h | 2 +-\n 3 files changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 87fa16b730..6ce3591c5b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3526,8 +3526,6 @@ static int set_diff_algorithm(struct diff_options *opts,\n \tif (value < 0)\n \t\treturn -1;\n \n-\t/* clear out previous settings */\n-\tDIFF_XDL_CLR(opts, NEED_MINIMAL);\n \topts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n \topts->xdl_opts |= value;\n \ndiff --git a/merge-ort.c b/merge-ort.c\nindex 29858074f9..9b2b0fce7e 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -5495,8 +5495,6 @@ int parse_merge_opt(struct merge_options *opt, const char *s)\n \t\tlong value = parse_algorithm_value(arg);\n \t\tif (value < 0)\n \t\t\treturn -1;\n-\t\t/* clear out previous settings */\n-\t\tDIFF_XDL_CLR(opt, NEED_MINIMAL);\n \t\topt->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n \t\topt->xdl_opts |= value;\n \t}\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 2cecde5afe..dc370712e9 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -43,7 +43,7 @@ extern \"C\" {\n \n #define XDF_PATIENCE_DIFF (1 << 14)\n #define XDF_HISTOGRAM_DIFF (1 << 15)\n-#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)\n+#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF | XDF_NEED_MINIMAL)\n #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)\n \n #define XDF_INDENT_HEURISTIC (1 << 23)\n-- \ngitgitgadget\n\n"},{"id":"530347","messageId":"60015bbadaf90f40b3b56d2e32b9f48818cb8675.1762468914.git.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v5.git.git.1762468914.gitgitgadget@gmail.com","subject":"[PATCH v5 2/2] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-06T22:41:54Z","receivedAt":"2025-11-06T22:41:59Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe diff algorithm used in 'git-blame(1)' is set to 'myers',\nwithout the possibility to change it aside from the `--minimal` option.\n\nThere has been long-standing interest in changing the default diff\nalgorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\nfor taking some steps towards that:\n\nhttps://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n\nAs a preparation for this move, it is worth making sure that the diff\nalgorithm is configurable where useful.\n\nMake it configurable in the `git-blame(1)` command by introducing the\n`--diff-algorithm` option and make honor the `diff.algorithm` config\nvariable. Keep Myers diff as the default.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n Documentation/diff-algorithm-option.adoc |  20 +++\n Documentation/diff-options.adoc          |  21 +--\n Documentation/git-blame.adoc             |   2 +\n builtin/blame.c                          |  52 +++++-\n t/meson.build                            |   1 +\n t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n 6 files changed, 278 insertions(+), 21 deletions(-)\n create mode 100644 Documentation/diff-algorithm-option.adoc\n create mode 100755 t/t8015-blame-diff-algorithm.sh\n\ndiff --git a/Documentation/diff-algorithm-option.adoc b/Documentation/diff-algorithm-option.adoc\nnew file mode 100644\nindex 0000000000..8e3a0b63d7\n--- /dev/null\n+++ b/Documentation/diff-algorithm-option.adoc\n@@ -0,0 +1,20 @@\n+`--diff-algorithm=(patience|minimal|histogram|myers)`::\n+\tChoose a diff algorithm. The variants are as follows:\n++\n+--\n+   `default`;;\n+   `myers`;;\n+\tThe basic greedy diff algorithm. Currently, this is the default.\n+   `minimal`;;\n+\tSpend extra time to make sure the smallest possible diff is\n+\tproduced.\n+   `patience`;;\n+\tUse \"patience diff\" algorithm when generating patches.\n+   `histogram`;;\n+\tThis algorithm extends the patience algorithm to \"support\n+\tlow-occurrence common elements\".\n+--\n++\n+For instance, if you configured the `diff.algorithm` variable to a\n+non-default value and want to use the default one, then you\n+have to use `--diff-algorithm=default` option.\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex ae31520f7f..9cdad6f72a 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -197,26 +197,7 @@ and starts with _<text>_, this algorithm attempts to prevent it from\n appearing as a deletion or addition in the output. It uses the \"patience\n diff\" algorithm internally.\n \n-`--diff-algorithm=(patience|minimal|histogram|myers)`::\n-\tChoose a diff algorithm. The variants are as follows:\n-+\n---\n-   `default`;;\n-   `myers`;;\n-\tThe basic greedy diff algorithm. Currently, this is the default.\n-   `minimal`;;\n-\tSpend extra time to make sure the smallest possible diff is\n-\tproduced.\n-   `patience`;;\n-\tUse \"patience diff\" algorithm when generating patches.\n-   `histogram`;;\n-\tThis algorithm extends the patience algorithm to \"support\n-\tlow-occurrence common elements\".\n---\n-+\n-For instance, if you configured the `diff.algorithm` variable to a\n-non-default value and want to use the default one, then you\n-have to use `--diff-algorithm=default` option.\n+include::diff-algorithm-option.adoc[]\n \n `--stat[=<width>[,<name-width>[,<count>]]]`::\n \tGenerate a diffstat. By default, as much space as necessary\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex e438d28625..adcbb6f5dc 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -85,6 +85,8 @@ include::blame-options.adoc[]\n \tIgnore whitespace when comparing the parent's version and\n \tthe child's to find where the lines came from.\n \n+include::diff-algorithm-option.adoc[]\n+\n --abbrev=<n>::\n \tInstead of using the default 7+1 hexadecimal digits as the\n \tabbreviated object name, use <m>+1 digits, where <m> is at\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 2703820258..27b513d27f 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n \t\t}\n \t}\n \n+\tif (!strcmp(var, \"diff.algorithm\")) {\n+\t\tlong diff_algorithm;\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tdiff_algorithm = parse_algorithm_value(value);\n+\t\tif (diff_algorithm < 0)\n+\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n+\t\t\t\t     var, value);\n+\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t\txdl_opts |= diff_algorithm;\n+\t\treturn 0;\n+\t}\n+\n \tif (git_diff_heuristic_config(var, value, cb) < 0)\n \t\treturn -1;\n \tif (userdiff_config(var, value) < 0)\n@@ -824,6 +837,38 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n \treturn 0;\n }\n \n+static int blame_diff_algorithm_minimal(const struct option *option,\n+\t\t\t\t\tconst char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\n+\tBUG_ON_OPT_ARG(arg);\n+\n+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n+\tif (!unset)\n+\t\t*opt |= XDF_NEED_MINIMAL;\n+\n+\treturn 0;\n+}\n+\n+static int blame_diff_algorithm_callback(const struct option *option,\n+\t\t\t\t\t const char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\tlong value = parse_algorithm_value(arg);\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tif (value < 0)\n+\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n+\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n+\n+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t*opt |= value;\n+\n+\treturn 0;\n+}\n+\n static int is_a_rev(const char *name)\n {\n \tstruct object_id oid;\n@@ -915,11 +960,16 @@ int cmd_blame(int argc,\n \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n+\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n+\t\t\t       N_(\"choose a diff algorithm\"),\n+\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n-\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n+\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n+\t\t\t       N_(\"spend extra cycles to find a better match\"),\n+\t\t\t       PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, blame_diff_algorithm_minimal),\n \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\ndiff --git a/t/meson.build b/t/meson.build\nindex 401b24e50e..9f2fe7af8b 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -955,6 +955,7 @@ integration_tests = [\n   't8012-blame-colors.sh',\n   't8013-blame-ignore-revs.sh',\n   't8014-blame-ignore-fuzzy.sh',\n+  't8015-blame-diff-algorithm.sh',\n   't8020-last-modified.sh',\n   't9001-send-email.sh',\n   't9002-column.sh',\ndiff --git a/t/t8015-blame-diff-algorithm.sh b/t/t8015-blame-diff-algorithm.sh\nnew file mode 100755\nindex 0000000000..55e1d540dc\n--- /dev/null\n+++ b/t/t8015-blame-diff-algorithm.sh\n@@ -0,0 +1,203 @@\n+#!/bin/sh\n+\n+test_description='git blame with specific diff algorithm'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tcat >file.c <<-\\EOF &&\n+\tint f(int x, int y)\n+\t{\n+\t  if (x == 0)\n+\t  {\n+\t    return y;\n+\t  }\n+\t  return x;\n+\t}\n+\n+\tint g(size_t u)\n+\t{\n+\t  while (u < 30)\n+\t  {\n+\t    u++;\n+\t  }\n+\t  return u;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x x >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Commit_1 git commit -m Commit_1 &&\n+\n+\tcat >file.c <<-\\EOF &&\n+\tint g(size_t u)\n+\t{\n+\t  while (u < 30)\n+\t  {\n+\t    u++;\n+\t  }\n+\t  return u;\n+\t}\n+\n+\tint h(int x, int y, int z)\n+\t{\n+\t  if (z == 0)\n+\t  {\n+\t    return x;\n+\t  }\n+\t  return y;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x A B C D x E F G >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Commit_2 git commit -m Commit_2\n+'\n+\n+test_expect_success 'blame uses Myers diff algorithm by default' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_2 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_2   while (u < 30)\n+\tCommit_1   {\n+\tCommit_2     u++;\n+\tCommit_1   }\n+\tCommit_2   return u;\n+\tCommit_1 }\n+\tCommit_1\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_1 {\n+\tCommit_2   if (z == 0)\n+\tCommit_1   {\n+\tCommit_2     return x;\n+\tCommit_1   }\n+\tCommit_2   return y;\n+\tCommit_1 }\n+\tEOF\n+\n+\tgit blame file.c > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors --diff-algorithm option' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit blame file.c --diff-algorithm histogram > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors diff.algorithm config variable' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit -c diff.algorithm=histogram blame file.c > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n+\t    -e \"s/ *$//g\" output > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame gives priority to --diff-algorithm over diff.algorithm' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n+\t    -e \"s/ *$//g\" output > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors --minimal option' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_2 A\n+\tCommit_2 B\n+\tCommit_2 C\n+\tCommit_2 D\n+\tCommit_1 x\n+\tCommit_2 E\n+\tCommit_2 F\n+\tCommit_2 G\n+\tEOF\n+\n+\tgit blame file.txt --minimal > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame respects the order of diff options' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_2 A\n+\tCommit_2 B\n+\tCommit_2 C\n+\tCommit_2 D\n+\tCommit_2 x\n+\tCommit_2 E\n+\tCommit_2 F\n+\tCommit_2 G\n+\tEOF\n+\n+\tgit blame file.txt --minimal --diff-algorithm myers > output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n"},{"id":"530378","messageId":"08a6c461-e162-4eee-a42d-1da8f05a0606@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v5.git.git.1762468914.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 0/2] blame: make diff algorithm configurable","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-07T15:49:56Z","receivedAt":"2025-11-07T15:50:02Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Antonin\n\nOn 06/11/2025 22:41, Antonin Delpeuch via GitGitGadget wrote:\n> Changes since v4:\n> \n>   * hide --minimal option\n>   * simplify tests to minimize spun processes\n>   * remove redundant XDF_NEED_MINIMAL in bit mask\n\nExcellent, the range-diff below looks good. Thanks for working on this\n\nPhillip\n\n> Antonin Delpeuch (2):\n>    xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n>    blame: make diff algorithm configurable\n> \n>   Documentation/diff-algorithm-option.adoc |  20 +++\n>   Documentation/diff-options.adoc          |  21 +--\n>   Documentation/git-blame.adoc             |   2 +\n>   builtin/blame.c                          |  52 +++++-\n>   diff.c                                   |   2 -\n>   merge-ort.c                              |   2 -\n>   t/meson.build                            |   1 +\n>   t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n>   xdiff/xdiff.h                            |   2 +-\n>   9 files changed, 279 insertions(+), 26 deletions(-)\n>   create mode 100644 Documentation/diff-algorithm-option.adoc\n>   create mode 100755 t/t8015-blame-diff-algorithm.sh\n> \n> \n> base-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v5\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v5\n> Pull-Request: https://github.com/git/git/pull/2075\n> \n> Range-diff vs v4:\n> \n>   1:  e81a5d2bd2 = 1:  e81a5d2bd2 xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n>   2:  920a6f3acb ! 2:  60015bbada blame: make diff algorithm configurable\n>       @@ builtin/blame.c: static int blame_move_callback(const struct option *option, con\n>        +\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n>        +\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n>        +\n>       -+\t*opt &= ~(XDF_NEED_MINIMAL | XDF_DIFF_ALGORITHM_MASK);\n>       ++\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n>        +\t*opt |= value;\n>        +\n>        +\treturn 0;\n>       @@ builtin/blame.c: int cmd_blame(int argc,\n>        -\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n>        +\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n>        +\t\t\t       N_(\"spend extra cycles to find a better match\"),\n>       -+\t\t\t       PARSE_OPT_NOARG, blame_diff_algorithm_minimal),\n>       ++\t\t\t       PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, blame_diff_algorithm_minimal),\n>         \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n>         \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n>         \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\n>       @@ t/t8015-blame-diff-algorithm.sh (new)\n>        +\tEOF\n>        +\n>        +\tgit -c diff.algorithm=histogram blame file.c > output &&\n>       -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n>       -+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n>       ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n>       ++\t    -e \"s/ *$//g\" output > actual &&\n>        +\ttest_cmp expected actual\n>        +'\n>        +\n>       @@ t/t8015-blame-diff-algorithm.sh (new)\n>        +\tCommit_2 }\n>        +\tEOF\n>        +\n>       -+\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram &&\n>       -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n>       -+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n>       ++\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram > output &&\n>       ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n>       ++\t    -e \"s/ *$//g\" output > actual &&\n>        +\ttest_cmp expected actual\n>        +'\n>        +\n> \n\n"},{"id":"530379","messageId":"xmqqh5v5hmat.fsf@gitster.g","threadId":"64356","inReplyTo":"e81a5d2bd23add19e04184f6b37910bc89a514a5.1762468914.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-07T15:52:58Z","receivedAt":"2025-11-07T15:53:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Antonin Delpeuch via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Antonin Delpeuch <antonin@delpeuch.eu>\n>\n> The XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience\n> and histogram diffs, not for the minimal one. This means that when\n> reseting the diff algorithm to the default one, one needs to separately\n> clear the bit for the minimal diff. There are places in the code that fail\n> to do that: merge-ort.c and builtin/merge-file.c.\n>\n> Add the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate\n> clearing of this bit in the places where it hasn't been forgotten.\n\nMakes sense.  In other words, lack of any algorithm-mask bit means\nthe code uses myers.\n\n> diff --git a/diff.c b/diff.c\n> index 87fa16b730..6ce3591c5b 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3526,8 +3526,6 @@ static int set_diff_algorithm(struct diff_options *opts,\n>  \tif (value < 0)\n>  \t\treturn -1;\n>  \n> -\t/* clear out previous settings */\n> -\tDIFF_XDL_CLR(opts, NEED_MINIMAL);\n>  \topts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n\nThe comment still accurately describes what the surviving line does,\nthough.  It is borderline if it needs commenting, but the topic of\nthis patch not being \"remove overly obvious comments\", I'd probably\nvote for retaining the comment.\n\n> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\n> index 2cecde5afe..dc370712e9 100644\n> --- a/xdiff/xdiff.h\n> +++ b/xdiff/xdiff.h\n> @@ -43,7 +43,7 @@ extern \"C\" {\n>  \n>  #define XDF_PATIENCE_DIFF (1 << 14)\n>  #define XDF_HISTOGRAM_DIFF (1 << 15)\n> -#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)\n> +#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF | XDF_NEED_MINIMAL)\n>  #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)\n\nGiven the definition of XDF_DIFF_ALG(), I wondered how it is used.\n\n    $ git grep -n -e 'XDF_DIFF_ALG(' \\*.c\n    xdiff/xdiffi.c:324:\tif (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) {\n    xdiff/xdiffi.c:329:\tif (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) {\n    xdiff/xprepare.c:170:\tif ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) &&\n    xdiff/xprepare.c:171:\t    (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) {\n    xdiff/xprepare.c:396:\tsample = (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF\n    xdiff/xprepare.c:417:\tif ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) &&\n    xdiff/xprepare.c:418:\t    (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) &&\n\nThey say \"if the specified algorithm is (or is not) patience (or\nhistogram), do this\".  Now the original code, because the mask did\nnot include the need-minimal bit, would have chosen patience code\npath even if xpp->flags had XDF_PATIENCE_DIFF and XDF_NEED_MINIMAL\nbits set at the same time.  The code would no longer do so.\n\nWhat is keeping us safe and not making this change a bug is that\namong XDF_DIFF_ALGORITHM_MASK bits, we intend to set at most one of\nthem at a time.  I wonder if we want the command line option and\nconfiguration parser to have an explicit check (and BUG(\"\")) to\nensure this constraint.\n\nAlso, in the longer term as #leftoverbits clean-up, perhaps these\nbits can be removed from xpp->flags and diff_options->xdl_opts, xpp\nstructure can gain a separate member that is an enum of the\nalgorithm names instead, and XDF_DIFF_ALGORITHM_MASK can be dropped?\n\nThen set_diff_algorithm() we saw earlier can become\n\n\tif (value < 0)\n\t\treturn -1;\n\topts->xdl_algo = value;\n\treturn 0;\n\nAnd since there is no \"clean out prvious settings\" required (now we\ncan simply overwrite), we can truly lose that old comment once we do\nso.\n\nAs a part of this topic, I think that a new code to sanity check\nthat there are at most one bit in XDF_DIFF_ALG(xpp->flags) may be a\ngood safety measure to have.  Moving the algorithm bits out of the\nflags is a larger change, and it may be better left outside the\ntopic, but I do not personaly mind seeing such a clean-up included\nas a preparatory change for this series, either.\n\nThanks.\n\n"},{"id":"530380","messageId":"xmqqbjldhm3o.fsf@gitster.g","threadId":"64356","inReplyTo":"60015bbadaf90f40b3b56d2e32b9f48818cb8675.1762468914.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 2/2] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-07T15:57:15Z","receivedAt":"2025-11-07T15:57:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Antonin Delpeuch via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Antonin Delpeuch <antonin@delpeuch.eu>\n>\n> The diff algorithm used in 'git-blame(1)' is set to 'myers',\n> without the possibility to change it aside from the `--minimal` option.\n>\n> There has been long-standing interest in changing the default diff\n> algorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\n> for taking some steps towards that:\n>\n> https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n>\n> As a preparation for this move, it is worth making sure that the diff\n> algorithm is configurable where useful.\n>\n> Make it configurable in the `git-blame(1)` command by introducing the\n> `--diff-algorithm` option and make honor the `diff.algorithm` config\n> variable. Keep Myers diff as the default.\n>\n> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n> ---\n\nThis step does not have anything surprising in it, knowing what the\nprevious iteration contained.  Looking good.\n\nOther than that many redirections into a file are written with a\nspace between redirection operator and its target, i.e.\n\n    command > output &&\n\nthat should be, according to the coding guidelines, written like\n\n    command >output &&\n\nthat is.\n\n> +test_expect_success 'blame respects the order of diff options' '\n> +\tcat >expected <<-\\EOF &&\n> +...\n> +\tEOF\n> +\n> +\tgit blame file.txt --minimal --diff-algorithm myers > output &&\n> +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n> +\ttest_cmp expected actual\n> +'\n\nThanks.\n"},{"id":"530784","messageId":"xmqq5xb9bgx2.fsf@gitster.g","threadId":"64356","inReplyTo":"08a6c461-e162-4eee-a42d-1da8f05a0606@gmail.com","subject":"Re: [PATCH v5 0/2] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-17T01:12:57Z","receivedAt":"2025-11-17T01:13:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Excellent, the range-diff below looks good. Thanks for working on this\n>\n> Phillip\n>\n>> Antonin Delpeuch (2):\n>>    xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n>>    blame: make diff algorithm configurable\n>> \n>>   Documentation/diff-algorithm-option.adoc |  20 +++\n>>   Documentation/diff-options.adoc          |  21 +--\n>>   Documentation/git-blame.adoc             |   2 +\n>>   builtin/blame.c                          |  52 +++++-\n>>   diff.c                                   |   2 -\n>>   merge-ort.c                              |   2 -\n>>   t/meson.build                            |   1 +\n>>   t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n>>   xdiff/xdiff.h                            |   2 +-\n>>   9 files changed, 279 insertions(+), 26 deletions(-)\n>>   create mode 100644 Documentation/diff-algorithm-option.adoc\n>>   create mode 100755 t/t8015-blame-diff-algorithm.sh\n\nOK, we haven't seen any activities since we saw this comment.  Are\nwe ready to mark the topic for 'next', or are you waiting for the\nend of feature freeze to make a (hopefully small and fanal) reroll?\n\nThanks.\n"},{"id":"530793","messageId":"pull.2075.v6.git.git.1763366672.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v5.git.git.1762468914.gitgitgadget@gmail.com","subject":"[PATCH v6 0/2] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-17T08:04:30Z","receivedAt":"2025-11-17T08:04:35Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"Changes since v5:\n\n * add back /* clear out previous settings */ comments\n * remove whitespace in bash output redirection\n\nAntonin Delpeuch (2):\n  xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n  blame: make diff algorithm configurable\n\n Documentation/diff-algorithm-option.adoc |  20 +++\n Documentation/diff-options.adoc          |  21 +--\n Documentation/git-blame.adoc             |   2 +\n builtin/blame.c                          |  52 +++++-\n diff.c                                   |   1 -\n merge-ort.c                              |   1 -\n t/meson.build                            |   1 +\n t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n xdiff/xdiff.h                            |   2 +-\n 9 files changed, 279 insertions(+), 24 deletions(-)\n create mode 100644 Documentation/diff-algorithm-option.adoc\n create mode 100755 t/t8015-blame-diff-algorithm.sh\n\n\nbase-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v6\nPull-Request: https://github.com/git/git/pull/2075\n\nRange-diff vs v5:\n\n 1:  e81a5d2bd2 ! 1:  4846715436 xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n     @@ Commit message\n      \n       ## diff.c ##\n      @@ diff.c: static int set_diff_algorithm(struct diff_options *opts,\n     - \tif (value < 0)\n       \t\treturn -1;\n       \n     --\t/* clear out previous settings */\n     + \t/* clear out previous settings */\n      -\tDIFF_XDL_CLR(opts, NEED_MINIMAL);\n       \topts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n       \topts->xdl_opts |= value;\n     @@ diff.c: static int set_diff_algorithm(struct diff_options *opts,\n      \n       ## merge-ort.c ##\n      @@ merge-ort.c: int parse_merge_opt(struct merge_options *opt, const char *s)\n     - \t\tlong value = parse_algorithm_value(arg);\n       \t\tif (value < 0)\n       \t\t\treturn -1;\n     --\t\t/* clear out previous settings */\n     + \t\t/* clear out previous settings */\n      -\t\tDIFF_XDL_CLR(opt, NEED_MINIMAL);\n       \t\topt->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n       \t\topt->xdl_opts |= value;\n 2:  60015bbada ! 2:  c477b87cc6 blame: make diff algorithm configurable\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_1 }\n      +\tEOF\n      +\n     -+\tgit blame file.c > output &&\n     -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n     -+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n     ++\tgit blame file.c >output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >without_varying_parts &&\n     ++\tsed -e \"s/ *$//g\" without_varying_parts >actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 }\n      +\tEOF\n      +\n     -+\tgit blame file.c --diff-algorithm histogram > output &&\n     -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n     -+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n     ++\tgit blame file.c --diff-algorithm histogram >output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >without_varying_parts &&\n     ++\tsed -e \"s/ *$//g\" without_varying_parts >actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 }\n      +\tEOF\n      +\n     -+\tgit -c diff.algorithm=histogram blame file.c > output &&\n     ++\tgit -c diff.algorithm=histogram blame file.c >output &&\n      +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n     -+\t    -e \"s/ *$//g\" output > actual &&\n     ++\t    -e \"s/ *$//g\" output >actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 }\n      +\tEOF\n      +\n     -+\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram > output &&\n     ++\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram >output &&\n      +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n     -+\t    -e \"s/ *$//g\" output > actual &&\n     ++\t    -e \"s/ *$//g\" output >actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 G\n      +\tEOF\n      +\n     -+\tgit blame file.txt --minimal > output &&\n     -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n     ++\tgit blame file.txt --minimal >output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n     @@ t/t8015-blame-diff-algorithm.sh (new)\n      +\tCommit_2 G\n      +\tEOF\n      +\n     -+\tgit blame file.txt --minimal --diff-algorithm myers > output &&\n     -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n     ++\tgit blame file.txt --minimal --diff-algorithm myers >output &&\n     ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >actual &&\n      +\ttest_cmp expected actual\n      +'\n      +\n\n-- \ngitgitgadget\n"},{"id":"530794","messageId":"48467154368ae0970f526d169528e4b199e690ed.1763366672.git.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v6.git.git.1763366672.gitgitgadget@gmail.com","subject":"[PATCH v6 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-17T08:04:31Z","receivedAt":"2025-11-17T08:04:36Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience\nand histogram diffs, not for the minimal one. This means that when\nreseting the diff algorithm to the default one, one needs to separately\nclear the bit for the minimal diff. There are places in the code that fail\nto do that: merge-ort.c and builtin/merge-file.c.\n\nAdd the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate\nclearing of this bit in the places where it hasn't been forgotten.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n diff.c        | 1 -\n merge-ort.c   | 1 -\n xdiff/xdiff.h | 2 +-\n 3 files changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 87fa16b730..cdcd11f1f7 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3527,7 +3527,6 @@ static int set_diff_algorithm(struct diff_options *opts,\n \t\treturn -1;\n \n \t/* clear out previous settings */\n-\tDIFF_XDL_CLR(opts, NEED_MINIMAL);\n \topts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n \topts->xdl_opts |= value;\n \ndiff --git a/merge-ort.c b/merge-ort.c\nindex 29858074f9..23e2b64c79 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -5496,7 +5496,6 @@ int parse_merge_opt(struct merge_options *opt, const char *s)\n \t\tif (value < 0)\n \t\t\treturn -1;\n \t\t/* clear out previous settings */\n-\t\tDIFF_XDL_CLR(opt, NEED_MINIMAL);\n \t\topt->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n \t\topt->xdl_opts |= value;\n \t}\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 2cecde5afe..dc370712e9 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -43,7 +43,7 @@ extern \"C\" {\n \n #define XDF_PATIENCE_DIFF (1 << 14)\n #define XDF_HISTOGRAM_DIFF (1 << 15)\n-#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)\n+#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF | XDF_NEED_MINIMAL)\n #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)\n \n #define XDF_INDENT_HEURISTIC (1 << 23)\n-- \ngitgitgadget\n\n"},{"id":"530795","messageId":"c477b87cc617f5302db40fc9a1a480f3392179b0.1763366672.git.gitgitgadget@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v6.git.git.1763366672.gitgitgadget@gmail.com","subject":"[PATCH v6 2/2] blame: make diff algorithm configurable","fromName":"Antonin Delpeuch via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-17T08:04:32Z","receivedAt":"2025-11-17T08:04:38Z","isPatch":true,"sender":{"key":"antonin@delpeuch.eu","avatar":"https://avatars.githubusercontent.com/u/309908?v=4"},"body":"From: Antonin Delpeuch <antonin@delpeuch.eu>\n\nThe diff algorithm used in 'git-blame(1)' is set to 'myers',\nwithout the possibility to change it aside from the `--minimal` option.\n\nThere has been long-standing interest in changing the default diff\nalgorithm to \"histogram\", and Git 3.0 was floated as a possible occasion\nfor taking some steps towards that:\n\nhttps://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/\n\nAs a preparation for this move, it is worth making sure that the diff\nalgorithm is configurable where useful.\n\nMake it configurable in the `git-blame(1)` command by introducing the\n`--diff-algorithm` option and make honor the `diff.algorithm` config\nvariable. Keep Myers diff as the default.\n\nSigned-off-by: Antonin Delpeuch <antonin@delpeuch.eu>\n---\n Documentation/diff-algorithm-option.adoc |  20 +++\n Documentation/diff-options.adoc          |  21 +--\n Documentation/git-blame.adoc             |   2 +\n builtin/blame.c                          |  52 +++++-\n t/meson.build                            |   1 +\n t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n 6 files changed, 278 insertions(+), 21 deletions(-)\n create mode 100644 Documentation/diff-algorithm-option.adoc\n create mode 100755 t/t8015-blame-diff-algorithm.sh\n\ndiff --git a/Documentation/diff-algorithm-option.adoc b/Documentation/diff-algorithm-option.adoc\nnew file mode 100644\nindex 0000000000..8e3a0b63d7\n--- /dev/null\n+++ b/Documentation/diff-algorithm-option.adoc\n@@ -0,0 +1,20 @@\n+`--diff-algorithm=(patience|minimal|histogram|myers)`::\n+\tChoose a diff algorithm. The variants are as follows:\n++\n+--\n+   `default`;;\n+   `myers`;;\n+\tThe basic greedy diff algorithm. Currently, this is the default.\n+   `minimal`;;\n+\tSpend extra time to make sure the smallest possible diff is\n+\tproduced.\n+   `patience`;;\n+\tUse \"patience diff\" algorithm when generating patches.\n+   `histogram`;;\n+\tThis algorithm extends the patience algorithm to \"support\n+\tlow-occurrence common elements\".\n+--\n++\n+For instance, if you configured the `diff.algorithm` variable to a\n+non-default value and want to use the default one, then you\n+have to use `--diff-algorithm=default` option.\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex ae31520f7f..9cdad6f72a 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -197,26 +197,7 @@ and starts with _<text>_, this algorithm attempts to prevent it from\n appearing as a deletion or addition in the output. It uses the \"patience\n diff\" algorithm internally.\n \n-`--diff-algorithm=(patience|minimal|histogram|myers)`::\n-\tChoose a diff algorithm. The variants are as follows:\n-+\n---\n-   `default`;;\n-   `myers`;;\n-\tThe basic greedy diff algorithm. Currently, this is the default.\n-   `minimal`;;\n-\tSpend extra time to make sure the smallest possible diff is\n-\tproduced.\n-   `patience`;;\n-\tUse \"patience diff\" algorithm when generating patches.\n-   `histogram`;;\n-\tThis algorithm extends the patience algorithm to \"support\n-\tlow-occurrence common elements\".\n---\n-+\n-For instance, if you configured the `diff.algorithm` variable to a\n-non-default value and want to use the default one, then you\n-have to use `--diff-algorithm=default` option.\n+include::diff-algorithm-option.adoc[]\n \n `--stat[=<width>[,<name-width>[,<count>]]]`::\n \tGenerate a diffstat. By default, as much space as necessary\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex e438d28625..adcbb6f5dc 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -85,6 +85,8 @@ include::blame-options.adoc[]\n \tIgnore whitespace when comparing the parent's version and\n \tthe child's to find where the lines came from.\n \n+include::diff-algorithm-option.adoc[]\n+\n --abbrev=<n>::\n \tInstead of using the default 7+1 hexadecimal digits as the\n \tabbreviated object name, use <m>+1 digits, where <m> is at\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 2703820258..27b513d27f 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -779,6 +779,19 @@ static int git_blame_config(const char *var, const char *value,\n \t\t}\n \t}\n \n+\tif (!strcmp(var, \"diff.algorithm\")) {\n+\t\tlong diff_algorithm;\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tdiff_algorithm = parse_algorithm_value(value);\n+\t\tif (diff_algorithm < 0)\n+\t\t\treturn error(_(\"unknown value for config '%s': %s\"),\n+\t\t\t\t     var, value);\n+\t\txdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t\txdl_opts |= diff_algorithm;\n+\t\treturn 0;\n+\t}\n+\n \tif (git_diff_heuristic_config(var, value, cb) < 0)\n \t\treturn -1;\n \tif (userdiff_config(var, value) < 0)\n@@ -824,6 +837,38 @@ static int blame_move_callback(const struct option *option, const char *arg, int\n \treturn 0;\n }\n \n+static int blame_diff_algorithm_minimal(const struct option *option,\n+\t\t\t\t\tconst char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\n+\tBUG_ON_OPT_ARG(arg);\n+\n+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n+\tif (!unset)\n+\t\t*opt |= XDF_NEED_MINIMAL;\n+\n+\treturn 0;\n+}\n+\n+static int blame_diff_algorithm_callback(const struct option *option,\n+\t\t\t\t\t const char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\tlong value = parse_algorithm_value(arg);\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tif (value < 0)\n+\t\treturn error(_(\"option diff-algorithm accepts \\\"myers\\\", \"\n+\t\t\t       \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\"));\n+\n+\t*opt &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t*opt |= value;\n+\n+\treturn 0;\n+}\n+\n static int is_a_rev(const char *name)\n {\n \tstruct object_id oid;\n@@ -915,11 +960,16 @@ int cmd_blame(int argc,\n \t\tOPT_BIT('s', NULL, &output_option, N_(\"suppress author name and timestamp (Default: off)\"), OUTPUT_NO_AUTHOR),\n \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n+\t\tOPT_CALLBACK_F(0, \"diff-algorithm\", &xdl_opts, N_(\"<algorithm>\"),\n+\t\t\t       N_(\"choose a diff algorithm\"),\n+\t\t\t       PARSE_OPT_NONEG, blame_diff_algorithm_callback),\n \t\tOPT_STRING_LIST(0, \"ignore-rev\", &ignore_rev_list, N_(\"rev\"), N_(\"ignore <rev> when blaming\")),\n \t\tOPT_STRING_LIST(0, \"ignore-revs-file\", &ignore_revs_file_list, N_(\"file\"), N_(\"ignore revisions from <file>\")),\n \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n-\t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n+\t\tOPT_CALLBACK_F(0, \"minimal\", &xdl_opts, NULL,\n+\t\t\t       N_(\"spend extra cycles to find a better match\"),\n+\t\t\t       PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, blame_diff_algorithm_minimal),\n \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"use revisions from <file> instead of calling git-rev-list\")),\n \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"use <file>'s contents as the final image\")),\n \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\ndiff --git a/t/meson.build b/t/meson.build\nindex 401b24e50e..9f2fe7af8b 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -955,6 +955,7 @@ integration_tests = [\n   't8012-blame-colors.sh',\n   't8013-blame-ignore-revs.sh',\n   't8014-blame-ignore-fuzzy.sh',\n+  't8015-blame-diff-algorithm.sh',\n   't8020-last-modified.sh',\n   't9001-send-email.sh',\n   't9002-column.sh',\ndiff --git a/t/t8015-blame-diff-algorithm.sh b/t/t8015-blame-diff-algorithm.sh\nnew file mode 100755\nindex 0000000000..cd709536c6\n--- /dev/null\n+++ b/t/t8015-blame-diff-algorithm.sh\n@@ -0,0 +1,203 @@\n+#!/bin/sh\n+\n+test_description='git blame with specific diff algorithm'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tcat >file.c <<-\\EOF &&\n+\tint f(int x, int y)\n+\t{\n+\t  if (x == 0)\n+\t  {\n+\t    return y;\n+\t  }\n+\t  return x;\n+\t}\n+\n+\tint g(size_t u)\n+\t{\n+\t  while (u < 30)\n+\t  {\n+\t    u++;\n+\t  }\n+\t  return u;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x x >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Commit_1 git commit -m Commit_1 &&\n+\n+\tcat >file.c <<-\\EOF &&\n+\tint g(size_t u)\n+\t{\n+\t  while (u < 30)\n+\t  {\n+\t    u++;\n+\t  }\n+\t  return u;\n+\t}\n+\n+\tint h(int x, int y, int z)\n+\t{\n+\t  if (z == 0)\n+\t  {\n+\t    return x;\n+\t  }\n+\t  return y;\n+\t}\n+\tEOF\n+\ttest_write_lines x x x A B C D x E F G >file.txt &&\n+\tgit add file.c file.txt &&\n+\tGIT_AUTHOR_NAME=Commit_2 git commit -m Commit_2\n+'\n+\n+test_expect_success 'blame uses Myers diff algorithm by default' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_2 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_2   while (u < 30)\n+\tCommit_1   {\n+\tCommit_2     u++;\n+\tCommit_1   }\n+\tCommit_2   return u;\n+\tCommit_1 }\n+\tCommit_1\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_1 {\n+\tCommit_2   if (z == 0)\n+\tCommit_1   {\n+\tCommit_2     return x;\n+\tCommit_1   }\n+\tCommit_2   return y;\n+\tCommit_1 }\n+\tEOF\n+\n+\tgit blame file.c >output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >without_varying_parts &&\n+\tsed -e \"s/ *$//g\" without_varying_parts >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors --diff-algorithm option' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit blame file.c --diff-algorithm histogram >output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >without_varying_parts &&\n+\tsed -e \"s/ *$//g\" without_varying_parts >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors diff.algorithm config variable' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit -c diff.algorithm=histogram blame file.c >output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n+\t    -e \"s/ *$//g\" output >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame gives priority to --diff-algorithm over diff.algorithm' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 int g(size_t u)\n+\tCommit_1 {\n+\tCommit_1   while (u < 30)\n+\tCommit_1   {\n+\tCommit_1     u++;\n+\tCommit_1   }\n+\tCommit_1   return u;\n+\tCommit_1 }\n+\tCommit_2\n+\tCommit_2 int h(int x, int y, int z)\n+\tCommit_2 {\n+\tCommit_2   if (z == 0)\n+\tCommit_2   {\n+\tCommit_2     return x;\n+\tCommit_2   }\n+\tCommit_2   return y;\n+\tCommit_2 }\n+\tEOF\n+\n+\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram >output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n+\t    -e \"s/ *$//g\" output >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame honors --minimal option' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_2 A\n+\tCommit_2 B\n+\tCommit_2 C\n+\tCommit_2 D\n+\tCommit_1 x\n+\tCommit_2 E\n+\tCommit_2 F\n+\tCommit_2 G\n+\tEOF\n+\n+\tgit blame file.txt --minimal >output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'blame respects the order of diff options' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_1 x\n+\tCommit_2 A\n+\tCommit_2 B\n+\tCommit_2 C\n+\tCommit_2 D\n+\tCommit_2 x\n+\tCommit_2 E\n+\tCommit_2 F\n+\tCommit_2 G\n+\tEOF\n+\n+\tgit blame file.txt --minimal --diff-algorithm myers >output &&\n+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n"},{"id":"530806","messageId":"fd03f2a5-bf9e-453f-97d1-d5a66bc87470@gmail.com","threadId":"64356","inReplyTo":"pull.2075.v6.git.git.1763366672.gitgitgadget@gmail.com","subject":"Re: [PATCH v6 0/2] blame: make diff algorithm configurable","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-17T14:13:12Z","receivedAt":"2025-11-17T14:13:15Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Antonin\n\nOn 17/11/2025 08:04, Antonin Delpeuch via GitGitGadget wrote:\n> Changes since v5:\n> \n>   * add back /* clear out previous settings */ comments\n>   * remove whitespace in bash output redirection\n\nThanks for re-rolling, the range-diff below looks good. Being able to \nconfigure the diff algorithm for \"git blame\" is a nice addition, thanks \nfor working on it.\n\nPhillip\n\n> Antonin Delpeuch (2):\n>    xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n>    blame: make diff algorithm configurable\n> \n>   Documentation/diff-algorithm-option.adoc |  20 +++\n>   Documentation/diff-options.adoc          |  21 +--\n>   Documentation/git-blame.adoc             |   2 +\n>   builtin/blame.c                          |  52 +++++-\n>   diff.c                                   |   1 -\n>   merge-ort.c                              |   1 -\n>   t/meson.build                            |   1 +\n>   t/t8015-blame-diff-algorithm.sh          | 203 +++++++++++++++++++++++\n>   xdiff/xdiff.h                            |   2 +-\n>   9 files changed, 279 insertions(+), 24 deletions(-)\n>   create mode 100644 Documentation/diff-algorithm-option.adoc\n>   create mode 100755 t/t8015-blame-diff-algorithm.sh\n> \n> \n> base-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2075%2Fwetneb%2Fblame_respects_diff_algorithm-v6\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2075/wetneb/blame_respects_diff_algorithm-v6\n> Pull-Request: https://github.com/git/git/pull/2075\n> \n> Range-diff vs v5:\n> \n>   1:  e81a5d2bd2 ! 1:  4846715436 xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n>       @@ Commit message\n>        \n>         ## diff.c ##\n>        @@ diff.c: static int set_diff_algorithm(struct diff_options *opts,\n>       - \tif (value < 0)\n>         \t\treturn -1;\n>         \n>       --\t/* clear out previous settings */\n>       + \t/* clear out previous settings */\n>        -\tDIFF_XDL_CLR(opts, NEED_MINIMAL);\n>         \topts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n>         \topts->xdl_opts |= value;\n>       @@ diff.c: static int set_diff_algorithm(struct diff_options *opts,\n>        \n>         ## merge-ort.c ##\n>        @@ merge-ort.c: int parse_merge_opt(struct merge_options *opt, const char *s)\n>       - \t\tlong value = parse_algorithm_value(arg);\n>         \t\tif (value < 0)\n>         \t\t\treturn -1;\n>       --\t\t/* clear out previous settings */\n>       + \t\t/* clear out previous settings */\n>        -\t\tDIFF_XDL_CLR(opt, NEED_MINIMAL);\n>         \t\topt->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n>         \t\topt->xdl_opts |= value;\n>   2:  60015bbada ! 2:  c477b87cc6 blame: make diff algorithm configurable\n>       @@ t/t8015-blame-diff-algorithm.sh (new)\n>        +\tCommit_1 }\n>        +\tEOF\n>        +\n>       -+\tgit blame file.c > output &&\n>       -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n>       -+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n>       ++\tgit blame file.c >output &&\n>       ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >without_varying_parts &&\n>       ++\tsed -e \"s/ *$//g\" without_varying_parts >actual &&\n>        +\ttest_cmp expected actual\n>        +'\n>        +\n>       @@ t/t8015-blame-diff-algorithm.sh (new)\n>        +\tCommit_2 }\n>        +\tEOF\n>        +\n>       -+\tgit blame file.c --diff-algorithm histogram > output &&\n>       -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > without_varying_parts &&\n>       -+\tsed -e \"s/ *$//g\" without_varying_parts > actual &&\n>       ++\tgit blame file.c --diff-algorithm histogram >output &&\n>       ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >without_varying_parts &&\n>       ++\tsed -e \"s/ *$//g\" without_varying_parts >actual &&\n>        +\ttest_cmp expected actual\n>        +'\n>        +\n>       @@ t/t8015-blame-diff-algorithm.sh (new)\n>        +\tCommit_2 }\n>        +\tEOF\n>        +\n>       -+\tgit -c diff.algorithm=histogram blame file.c > output &&\n>       ++\tgit -c diff.algorithm=histogram blame file.c >output &&\n>        +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n>       -+\t    -e \"s/ *$//g\" output > actual &&\n>       ++\t    -e \"s/ *$//g\" output >actual &&\n>        +\ttest_cmp expected actual\n>        +'\n>        +\n>       @@ t/t8015-blame-diff-algorithm.sh (new)\n>        +\tCommit_2 }\n>        +\tEOF\n>        +\n>       -+\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram > output &&\n>       ++\tgit -c diff.algorithm=myers blame file.c --diff-algorithm histogram >output &&\n>        +\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" \\\n>       -+\t    -e \"s/ *$//g\" output > actual &&\n>       ++\t    -e \"s/ *$//g\" output >actual &&\n>        +\ttest_cmp expected actual\n>        +'\n>        +\n>       @@ t/t8015-blame-diff-algorithm.sh (new)\n>        +\tCommit_2 G\n>        +\tEOF\n>        +\n>       -+\tgit blame file.txt --minimal > output &&\n>       -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n>       ++\tgit blame file.txt --minimal >output &&\n>       ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >actual &&\n>        +\ttest_cmp expected actual\n>        +'\n>        +\n>       @@ t/t8015-blame-diff-algorithm.sh (new)\n>        +\tCommit_2 G\n>        +\tEOF\n>        +\n>       -+\tgit blame file.txt --minimal --diff-algorithm myers > output &&\n>       -+\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output > actual &&\n>       ++\tgit blame file.txt --minimal --diff-algorithm myers >output &&\n>       ++\tsed -e \"s/^[^ ]* (\\([^ ]*\\) [^)]*)/\\1/g\" output >actual &&\n>        +\ttest_cmp expected actual\n>        +'\n>        +\n> \n\n"},{"id":"530828","messageId":"xmqqtsysikl6.fsf@gitster.g","threadId":"64356","inReplyTo":"pull.2075.v6.git.git.1763366672.gitgitgadget@gmail.com","subject":"Re: [PATCH v6 0/2] blame: make diff algorithm configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-17T18:24:05Z","receivedAt":"2025-11-17T18:24:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Antonin Delpeuch via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> Changes since v5:\n>\n>  * add back /* clear out previous settings */ comments\n>  * remove whitespace in bash output redirection\n>\n> Antonin Delpeuch (2):\n>   xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK\n>   blame: make diff algorithm configurable\n\nWill queue.  Thanks!\n"}]}