Re: [PATCH v2 00/11] Fix color handling in git add -i

From: Jeff King <peff@peff.net>
Date: Nov 17, 2020, 01:49 UTC
Message-ID: <20201117014937.GB19433@coredump.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.2011160036100.18437@tvgsbejvaqbjf.bet>

On Mon, Nov 16, 2020 at 12:40:32AM +0100, Johannes Schindelin wrote:

Show 12 quoted lines

> > All of which is to say that the current state is a bit of a mess, and I
> > don't consider it completely necessary to fix it before the builtin
> > version becomes the default. But:
> >
> >   - I think we should expect some possible complaints / bug reports
> >     from fringe users of the already-somewhat-broken state
> >
> >   - IMHO the parsing of the filtered version done by the builtin is
> >     going in the wrong direction (see my other mail for an elaboration)
> 
> It's a little bit of a surprise that this is the first time I hear about
> this.

Well, it is the first time I am finding out about it, too. ;)

> The way _I_ would go about fixing this is to look for a tell-tale that
> we're looking at a `diff-so-fancy` style output, e.g. by looking for that
> horizontal line, then adding special code to handle that.

IMHO that's the wrong direction. My intent with the feature was to give filters as much flexibility as possible, and beyond having 1-to-1 correspondence of lines, we don't know or care what they produce. We don't even know what other filters might exist out there (though I would be surprised if there is anything more exotic than intra-line coloring; other folks have pointed to "delta" for this use, but only in its color-only mode).

If anybody wants to put work into it, I think a better direction is having Git keep its own colored hunks and just present single-hunks to a custom display command to show to the user. Then we can continue not to care about the format of the individual filters, _and_ they can get the opportunity to format our rewritten split-hunk headers, etc.

Show 5 quoted lines

> But given that it does not work right now, and given that it has not been
> working for a long time, I think it does not hurt so much if it is left in
> the current state for a bit longer. I would love to focus on the patch
> series that kicked off this discussion first, getting it done, before I
> think about other `add -p` aspects.

Yeah, absolutely.

-Peff