Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 19, 2016, 13:09 UTC
Message-ID: <alpine.DEB.2.20.1608191500470.4924@virtualbox>
In-Reply-To: <20160818220530.2dcsag4qeitia4ao@sigill.intra.peff.net>

Hi Peff,

On Thu, 18 Aug 2016, Jeff King wrote:

Show 19 quoted lines

> On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
> 
> > With this patch, --batch can be combined with --textconv or --filters.
> > For this to work, the input needs to have the form
> > 
> > 	<object name><single white space><path>
> > 
> > so that the filters can be chosen appropriately.
> 
> The object name can have spaces in it, too. E.g.:
> 
>   HEAD:path with spaces
> 
> or even:
> 
>   :/grep for this
> 
> (as was pointed out to me when I tried to turn on %(rest) handling by
> default, long ago). How do those work with your patch?

They don't ;-)

And quite frankly, the documentation should make it clear to users that batch mode with --filters or --textconv won't work when the object name contains white space: it says that the object name is split from the path at the first white space character.

> It looks like the extra split isn't enabled unless one of those options
> is selected. Since --filters is new, that's OK for backwards
> compatibility. But --textconv isn't.

Except that it is okay, because --textconv *was not even supported in batch mode*. So there is no backwards compatibility that could be broken.

Show 5 quoted lines

> I wonder if we need an option specifically to say "the object name can
> be split". Right now it kicks in automatically if you use "%(rest)" in
> your format, but you might not care about passing along that output
> (e.g., a lot of times I am piping "rev-list" straight to cat-file, and I
> have to use a separate "cut" to throw away the pathnames).

As I said elsewhere in this thread: if anybody encounters that problem, they are welcome to fix it themselves.

My patch series purpose is to introduce --filters and make it compatible with the batch mode. As I imitated --textconv, it was both easy and correct to make it compatible with the batch mode, too. But that is the extent of this here patch series.

Fixing %(rest) for object names containing spaces is distinctly outside the original intent, and certainly outside of my use case.

Ciao, Dscho