Re: [PATCH v3] blame: make diff algorithm configurable
- From
Antonin Delpeuch <antonin@delpeuch.eu>
- Date
- Oct 30, 2025, 09:22 UTC
- Message-ID
- <33d44dc6-36b3-4736-b3ed-96861a3c4003@delpeuch.eu>
- In-Reply-To
- <fde3dae1-bb11-45e8-9211-50ae003ca497@gmail.com>
Hi Phillip,
On 29/10/2025 11:16, Phillip Wood wrote:
Show 14 quoted lines
> Unfortunately XDF_DIFF_ALGORITHM_MASK does not include > XDF_NEED_MINIMAL so if the user has a config file that looks like > > [diff] > algorithm = minimal > algorithm = myers > > We'll parse it as "minimal" rather than "myers" > > As we need to reset the diff algorithm in a number of places I think > it would be best to define a macro > > #define CLEAR_DIFF_ALGORITHM(flags) \ > flags &= ~(XDF_DIFF_ALGORITHM_MASK | XDF_NEED_MINIMAL)
Ouch, good catch! This problem is affecting other places as well.
I'm wondering if we couldn't even add XDF_NEED_MINIMAL to XDF_DIFF_ALGORITHM_MASK. I've reviewed all the places where XDF_DIFF_ALGORITHM_MASK is used, and it seems that in all cases it would either preserve the existing behaviour (potentially allowing us to remove an accompanying "DIFF_XDL_CLR(opts, NEED_MINIMAL);" macro which becomes redundant), or in some other cases it would fix a similar issue (for instance, in merge-file.c).
Is your suggestion to introduce a new macro motivated by stability concerns? I'm aware that xdiff is used in other code bases as a library, so I guess changing XDF_DIFF_ALGORITHM_MASK can indeed be seen as a breaking change.
Antonin