Thread (20 messages) 20 messages, 3 authors, 2025-11-17

Re: [PATCH v3] blame: make diff algorithm configurable

From: Antonin Delpeuch <hidden>
Date: 2025-10-30 09:35:49

Hi Phillip,

On 29/10/2025 11:16, Phillip Wood wrote:
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help