From: Phillip Wood <phillip.wood123@gmail•com>
To: Antonin Delpeuch via GitGitGadget <gitgitgadget@gmail•com>,
git@vger•kernel.org
Cc: Elijah Newren <newren@gmail•com>, Antonin Delpeuch <antonin@delpeuch•eu>
Subject: Re: [PATCH v4 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK
Date: Mon, 3 Nov 2025 14:32:33 +0000 [thread overview]
Message-ID: <6ec34cb1-4149-48b3-8c15-fe3460aae729@gmail.com> (raw)
In-Reply-To: <e81a5d2bd23add19e04184f6b37910bc89a514a5.1762034252.git.gitgitgadget@gmail.com>
Hi Antonin
On 01/11/2025 21:57, Antonin Delpeuch via GitGitGadget wrote:
> From: Antonin Delpeuch <antonin@delpeuch•eu>
>
> The XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience
> and histogram diffs, not for the minimal one. This means that when
> reseting the diff algorithm to the default one, one needs to separately
> clear the bit for the minimal diff. There are places in the code that fail
> to do that: merge-ort.c and builtin/merge-file.c.
>
> Add the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate
> clearing of this bit in the places where it hasn't been forgotten.
Nicely explained. This is a useful improvement that should prevent
errors in the future. After this patch there are no users of
DIFF_XDL_CLR() so we should probably remove that macro. I'm not sure it
makes sense to remove the comments that have been deleted below as we're
still clearing the old setting. Apart from that this all looks good.
Thanks
Phillip
> Signed-off-by: Antonin Delpeuch <antonin@delpeuch•eu>
> ---
> diff.c | 2 --
> merge-ort.c | 2 --
> xdiff/xdiff.h | 2 +-
> 3 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 87fa16b730..6ce3591c5b 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3526,8 +3526,6 @@ static int set_diff_algorithm(struct diff_options *opts,
> if (value < 0)
> return -1;
>
> - /* clear out previous settings */
> - DIFF_XDL_CLR(opts, NEED_MINIMAL);
> opts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
> opts->xdl_opts |= value;
>
> diff --git a/merge-ort.c b/merge-ort.c
> index 29858074f9..9b2b0fce7e 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -5495,8 +5495,6 @@ int parse_merge_opt(struct merge_options *opt, const char *s)
> long value = parse_algorithm_value(arg);
> if (value < 0)
> return -1;
> - /* clear out previous settings */
> - DIFF_XDL_CLR(opt, NEED_MINIMAL);
> opt->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
> opt->xdl_opts |= value;
> }
> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
> index 2cecde5afe..dc370712e9 100644
> --- a/xdiff/xdiff.h
> +++ b/xdiff/xdiff.h
> @@ -43,7 +43,7 @@ extern "C" {
>
> #define XDF_PATIENCE_DIFF (1 << 14)
> #define XDF_HISTOGRAM_DIFF (1 << 15)
> -#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)
> +#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF | XDF_NEED_MINIMAL)
> #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)
>
> #define XDF_INDENT_HEURISTIC (1 << 23)
next prev parent reply other threads:[~2025-11-03 14:32 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-20 14:56 [PATCH] blame: make diff algorithm configurable Antonin Delpeuch via GitGitGadget
2025-10-20 16:05 ` Junio C Hamano
2025-10-22 9:37 ` Antonin Delpeuch
2025-10-22 20:39 ` Junio C Hamano
2025-10-23 16:03 ` Phillip Wood
2025-10-28 13:37 ` [PATCH v2] " Antonin Delpeuch via GitGitGadget
2025-10-28 15:22 ` Junio C Hamano
2025-10-28 16:00 ` Antonin Delpeuch
2025-10-28 21:14 ` [PATCH v3] " Antonin Delpeuch via GitGitGadget
2025-10-29 10:16 ` Phillip Wood
2025-10-29 18:46 ` Junio C Hamano
2025-10-30 9:22 ` Antonin Delpeuch
2025-10-30 10:47 ` Phillip Wood
2025-11-01 21:57 ` [PATCH v4 0/2] " Antonin Delpeuch via GitGitGadget
2025-11-01 21:57 ` [PATCH v4 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK Antonin Delpeuch via GitGitGadget
2025-11-03 14:32 ` Phillip Wood [this message]
2025-11-01 21:57 ` [PATCH v4 2/2] blame: make diff algorithm configurable Antonin Delpeuch via GitGitGadget
2025-11-03 14:32 ` Phillip Wood
2025-11-03 16:15 ` Junio C Hamano
2025-11-06 20:29 ` Junio C Hamano
2025-11-06 22:41 ` [PATCH v5 0/2] " Antonin Delpeuch via GitGitGadget
2025-11-06 22:41 ` [PATCH v5 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK Antonin Delpeuch via GitGitGadget
2025-11-07 15:52 ` Junio C Hamano
2025-11-06 22:41 ` [PATCH v5 2/2] blame: make diff algorithm configurable Antonin Delpeuch via GitGitGadget
2025-11-07 15:57 ` Junio C Hamano
2025-11-07 15:49 ` [PATCH v5 0/2] " Phillip Wood
2025-11-17 1:12 ` Junio C Hamano
2025-11-17 8:04 ` [PATCH v6 " Antonin Delpeuch via GitGitGadget
2025-11-17 8:04 ` [PATCH v6 1/2] xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK Antonin Delpeuch via GitGitGadget
2025-11-17 8:04 ` [PATCH v6 2/2] blame: make diff algorithm configurable Antonin Delpeuch via GitGitGadget
2025-11-17 14:13 ` [PATCH v6 0/2] " Phillip Wood
2025-11-17 18:24 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6ec34cb1-4149-48b3-8c15-fe3460aae729@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=antonin@delpeuch$(echo .)eu \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=newren@gmail$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox