From: Junio C Hamano Date: Sun, 19 Dec 2010 21:59:50 GMT Subject: Re: [PATCH] convert filter: supply path to external driver Message-ID: <7vzks1e84p.fsf@alter.siamese.dyndns.org> In-Reply-To: <20101219212925.GA7393@arf.padd.com> Pete Wyckoff writes: > Filtering to support keyword expansion may need the name of > the file being filtered. In particular, to support p4 keywords > like > > $File: //depot/product/dir/script.sh $ > > the smudge filter needs to know the name of the file it is > smudging. > > Add a "%s" conversion specifier to the gitattribute for filter. > It will be expanded with the path name to the file when invoking > the external filter command. > > Signed-off-by: Pete Wyckoff > --- > > pw@padd.com wrote on Sat, 18 Dec 2010 17:38 -0500: >> I'm using git-p4 to import and work with upstream p4 >> repositories. Some of the files are ktext, meaning they expect >> expansion of $Id$ and similar identifiers. >> >> Using the filter driver for this file, I can do the "clean" part >> easily, but to calculate the "smudge" correctly, I need to know >> the filename inside the filter driver. > > This works fine for me. It is backward compatible, and leaves > open the possibility of adding other % modifiers if we find > a need later. This is not backward compatible for people who wanted to use '%' literal on their filter command line for whatever reason, so please do not advertise as such. A fair argument you could make is "Even though this is not strictly backward compatible, it is very unlikely that people passed a literal % to their filter command line, and the benefit of being able to give the pathname information would outweigh the downside of not being compatible", and people can agree or disagree. I am personally moderately negative about $any expansion$ (I don't use it myself, and I don't think sane people use it either). As far as I can tell, this should has no impact on the correctness and very little impact on the performance for people who do not use $any expansion$, so I am Ok with the patch. Modulo one worry. Don't we have, or don't we at least plant to allow us to have, a facility to cache expensive blob conversion result, similar to the textconv caching? How would this change interact with two blobs that live in different paths?