Re: [PATCH 3/4] add: introduce '--resolved' option

Michael Montalbo <[email protected]> Sun, 2 Aug 2026 18:15:20 -0700
Newsgroups org.kernel.vger.git
Message-ID <CAC2Qwm+4qnAdUpinbda+etnFYUaYkycW-0HasHo95MM3rPY3yA@mail.gmail.com>
On Sun, Aug 2, 2026 at 3:38=E2=80=AFPM Junio C Hamano <[email protected]> w=
rote:
>
>
> If we misidentify early, that is a desirable outcome, isn't it?
>
> We did not have to scan much and we gave control back to the user as
> soon as we saw "<<<<<<", telling them that we refused to add the path
> to the index, so that the user can inspect the situation more
> deeply.  In this application, false positives are much better than
> false negatives, and failing early is better than failing late.
>

I didn't fully consider the case of the false negative that would come
with switching the order of the checks. I agree false positives are
much better in this scenario.

> So, no, I do not think so, even though in practice I do not think it
> would matter either way.
>
> A question that may have much more impact is whether the attribute
> system should have any say in this code path.  I am somewhat torn on
> this.

I do think something in that direction might make things more clear. When
looking at "has_conflict_markers()" in isolation my instinct was to guard
against the case that the file is binary (among other "easy" checks to
eliminate non-conflict marker paths upfront) and return right away. That re=
sted
on the assumption the check for a binary file had an unambiguous result as
opposed to being a heuristic.

Would it be possible to check the merge driver used and return early if a
non-text driver was used? I tried tracing the path where conflict markers e=
nd
up being generated and it seems like the binary and union drivers cannot
emit conflict markers. That might be conceptually more straightforward and
eliminate the need for a "buffer_is_binary()" check. Not sure if that would=
 be
brittle in the face of new merge drivers being added, but directly relating
this conflict marker check with the drivers that emit those markers seems t=
o
make sense to me.