Re: [PATCH 2/2] add--interactive: detect bogus diffFilter output

From: Jeff King <peff@peff.net>
Date: Mar 5, 2018, 20:56 UTC
Message-ID: <20180305205626.GD5953@sigill.intra.peff.net>
In-Reply-To: <xmqqsh9e8css.fsf@gitster-ct.c.googlers.com>

On Mon, Mar 05, 2018 at 12:53:07PM -0800, Junio C Hamano wrote:

Show 17 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > It's important that the diff-filter only filter the
> > individual lines, and that there remain a one-to-one mapping
> > between the input and output lines. Otherwise, things like
> > hunk-splitting will behave quite unexpectedly (e.g., you
> > think you are splitting at one point, but it has a different
> > effect in the text patch we apply).
> >
> > We can't detect all problematic cases, but we can at least
> > catch the obvious case where we don't even have the correct
> > number of lines.
> 
> Will queue.  We could probably also make sure that each of the
> corresponding line pair begins with the same '-/ /+' letter, but we
> need to draw a line and stop somewhere, and I think the number of
> lines is probably a good enough place.

I think that would break things like diff-so-fancy, which actually removes the -/+ in favor of pure coloring (not something I care for myself, but it seems a legitimate use case). So it's probably best not to look at the content, not just from a "we can only go so far" perspective but also because we actively don't know what the filter's output is supposed to look like.

-Peff