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.