Re: [PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Wed, 24 Aug 2016, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Mentioned elsewhere, but I think the above should be
> >
> > 	if (!path)
> >         	path = obj_context.path;
> >
> > 	if (obj_context.mode == S_IFINVALID)
> >         	obj_context.mode = 0100644;
> >
> > IOW, even when there is an explicit path supplied, we should fall
> > back to assumed "regular blob" mode, so that
> >
> > 	git cat-file --filters --path=README $(git rev-parse :README)
> >
> > would work as expected.
> 
> Actually, I am reading the conditional the other way, but the
> conclusion "defaulting from unknown mode to regular blob is
> necessary whether the user gave us a path or not" is the same.
> 
> The current code may fail if --path is not available and 40-hex that
> does not give us any context of look up is given because it won't be
> able to decide how to filter, so using "else if" would not have
> practical difference there, but conceptually it still is wrong.

Let's translate the logic

	if (!path)
		path = obj_context.path;
	else if (obj_context.mode == S_IFINVALID)
		obj_context.mode = 0100644;

into plain English.

Basically, we have two cases:

1) the user provided us with a --path option, in which case we only
   override the mode if get_sha1_with_context() could not determine the
   mode.

2) the user did *not* provide us with a --path, in which case we keep the
   mode exactly as determined by get_sha1_with_context().

Now, the change you propose would change 2) such that we would *still*
override an undecided mode with 100644, even if the user did not bother to
specify a --path option.

It is true that it is currently impossible to infer a path from a blob ID
without being able to infer also a mode. So the question is whether it
makes sense to allow cat-file to proceed on the assumption that it is a
regular file if it does not know?

I would say: no, it does not make sense.

However, I do not want to hold this patch series up just by being
stubborn.

Will change it,
Dscho