Re: [PATCH 5/5] combine-diff: respect textconv attributes
- From
Jeff King <peff@peff.net>
- Date
- May 23, 2011, 23:39 UTC
- 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:
Show 11 quoted lines
> 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