Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option

[John Keeping <john@keeping.me.uk>]

On Tue, Apr 09, 2013 at 01:57:21PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > This adds a prefix string to any filename arguments encountered after it
> > has been specified.
> >
> > Signed-off-by: John Keeping <john@keeping.me.uk>
> > ---
> 
> Stale subject?

Yep.  Sorry.

> > +--prefix <arg>::
> > +	Behave as if 'git rev-parse' was invoked from the `<arg>`
> > +	subdirectory of the working tree.  Any relative filenames are
> > +	resolved as if they are prefixed by `<arg>` and will be printed
> > +	in that form.
> > ++
> > +This can be used to convert arguments to a command run in a subdirectory
> > +so that they can still be used after moving to the top-level of the
> > +repository.  For example:
> > ++
> > +----
> > +prefix=$(git rev-parse --show-prefix)
> > +cd "$(git rev-parse --show-toplevel)"
> > +eval "set -- $(git rev-parse --sq --prefix "$prefix" "$@")"
> 
> I think you should tighten rev-parse parameter to reject options and
> revisions, especially as an example to teach how to use it.  When
> the user said
> 
> 	git mylog -U20 master..next -- README
> 
> inside Documentation/ directory,  "git-mylog" script would want to
> see README prefixed with Documentation/ but want to see -U20 or
> master..next untouched.

And it will if it runs:

	git rev-parse --prefix Documentation/ -U20 master..next -- README

Which gives:

	-U20
	f131fb6eb2a1e09f7ce53d148e21ce6960f42422
	^fa7285dc3dce8bd01fd8c665b032603ed55348e5
	--
	Documentation/README

>                          Historically, rev-parse was a way to sift
> options and args meant for rev-list from those mant for diff-tree
> so that a variant of
> 
> 	git rev-list $(git rev-parse --revs) "$@" |
>         git diff-tree --stdin $(git rev-parse --no-revs)
> 
> can be used to implement such "git mylog" script.
> 
> I think "--no-revs --no-flags" is how you ask it to give you only
> the paths, but I am writing from memory so please double check.
> 
> Having said all that.
> 
> Existing scripts (e.g. "git am") do this kind of things without such
> an option added to rev-parse.  They first do:
> 
>     prefix=$(git rev-parse --show-prefix)
> 
> and refer to "$prefix/$1", "$prefix/$2", etc., I think.
> 
> Is this option really necessary to update "git submodule"?  Don't we
> have a much better idea which parameter holds user-supplied path in the
> script than having rev-parse make a guess on the entire "$@"?

It's not guessing on all of "$@" in git-submodule - we know that
everything left is a path.

I've looked at git-am and hadn't thought of doing that, just thought
this was a reasonably elegant way of processing the arguments.  I'm
happy to try another approach if that's going to be more acceptable.