Re: [PATCH 5/5] combine-diff: respect textconv attributes

From: Junio C Hamano <gitster@pobox.com>
Date: May 24, 2011, 16:20 UTC
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.