From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 24 May 2011 16:20:53 GMT
Subject: Re: [PATCH 5/5] combine-diff: respect textconv attributes
Message-ID: <7vk4dg83yy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110523203105.GE6298@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> This patch converts file contents according to textconv attributes. The
> implementation is slightly ugly; because the textconv code is tightly
> linked with the diff_filespec code, we temporarily create a
> diff_filespec during conversion.

After reading this patch again, I think this aversion to diff_filespec is
probably unjustified.  It is primarily a structure that records what path
has which blob by recording its object name and what mode, and also
in-core data when we make the contents available. There are small diff
specific data associated with it, but that could be separated out if you
really wanted to, perhaps like:

	struct diff_filespec {
        	struct filespec {
			... the generic and essential part ...
                } spec;
                /* diff specific part follows */
                const char *funcname_pattern_ident;
                int xfrm_flags;
                int rename_used;
                unsigned dirty_submodule :2;
                struct userdiff_driver *driver;
                int is_binary;
	};

and let most users use "struct filespec".

If anything else, we should be using the type in _more_ codepaths that are
not diff related but want to represent a path with its contents in the git
namespace (be it from working tree, index or a tree), not less, and in the
longer term weaken functions like fill_textconv() that take diff_filespec
to take filespec so that they can be made more reusable.