From: Jeff King <peff@peff.net>
Date: Mon, 23 May 2011 23:39:16 GMT
Subject: Re: [PATCH 5/5] combine-diff: respect textconv attributes
Message-ID: <20110523233916.GA10488@sigill.intra.peff.net>
In-Reply-To: <7vwrhht4oj.fsf@alter.siamese.dyndns.org>

On Mon, May 23, 2011 at 03:47:40PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > +	} else if (textconv) {
> > +		/* yuck, the textconv code is linked heavily with
> > +		 * filespecs */
> > +		struct diff_filespec *df = alloc_filespec("");
> > +		fill_filespec(df, sha1, mode);
> > +		*size = fill_textconv(textconv, df, &blob);
> 
> I thought your 4/5 talked something about not requiring filespec for
> textconv handling...?  Is it still yuck?

Yeah, I removed the requirement for get_textconv to need a filespec.
Which is nice, because it means the non-textconv codepath doesn't deal
with this at all.

But removing the filespec from fill_textconv is much nastier, because it
relies on diff.c's prepare_temp_file, which also uses the filespec.

So it's still yuck, but limited to this little conditional. It would be
nice if we used filespecs all the way through the combine-diff code
path, but that was a bit more refactoring than I was up for today.

-Peff