Re: [PATCH] diff: clarify textconv interface

From: Duy Nguyen <pclouds@gmail.com>
Date: Feb 22, 2016, 23:01 UTC
Message-ID: <CACsJy8Cg4cgCEqwo=9ujdTV8aw5CO-irVxOCanzBG4n7Nm-0kg@mail.gmail.com>
In-Reply-To: <20160222182854.GA11732@sigill.intra.peff.net>

On Tue, Feb 23, 2016 at 1:28 AM, Jeff King <peff@peff.net> wrote:

Show 14 quoted lines

> On Mon, Feb 22, 2016 at 01:06:45PM -0500, Jeff King wrote:
>
>> I have a feeling you were confused by the fact that fill_textconv()
>> does:
>
> Looking over it, I agree this is a pretty confusing interface that grew
> out of control over time.  But refactoring it is kind of tricky, because
> we really do want to avoid extra allocations, or cross-module
> assumptions (e.g., userdiff doesn't know about diff_filespec, but rather
> the other way around, and we probably do not want to muck with the
> internals of a diff_filespec when doing a textconv).
>
> So I think the patch below is an improvement, but if somebody really
> wants to dig into refactoring it, be my guest.

I almost went this way but I wasn't sure if driver->textconv can be NULL on purpose. Definitely an improvement.

-- 
Duy