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

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

On Sun, Apr 07, 2013 at 03:14:58PM -0700, Jonathan Nieder wrote:
> John Keeping wrote:
> 
> > This adds a prefix string to any filename arguments encountered after it
> > has been specified.
> 
> I assume this is a way of passing the prefix in?  In that case, I
> think a good UI would be
> 
> 	git rev-parse --prefix=Documentation/ <usual rev-parse args>
> 
> That sounds like a useful thing and would make the meaning very clear.

Yes (ish), the intended usage is something like this:

    prefix=$(git rev-parse --show-prefix)
    cd_to_toplevel
    ... parse options here ...
    # Convert remaining arguments (filenames) into top-level paths:
    eval "set $(git rev-parse --prefix "$prefix" --sq -- "$@")"

The "ish" is that my current implementation introduced a new variable
instead of simply resetting the existing "prefix" variable, which I
assume is what you mean.  That is probably simpler than my
implementation, but loses the ability to be at an intermediate level,
for example:

    cd Documentation/
    eval "set $(git rev-parse --prefix technical/ --sq -- api-strbuf.txt)"

> How does this interact with the following options?
> 
>  * --resolve-git-dir some/relative/path

It doesn't change this since --resolve-git-dir is handled separately
from the other argument parsing at the moment and you cannot specify
any other options with it.

>  * master:./path

I hadn't considered this case, but I think it should be inserting the
prefix into the path.  I suspect the easiest thing to do is simply make
the path part of that absolute, by combining both the prefix based on
$PWD and the prefix specified on the command line, but I haven't looked
at doing this yet.

The other think that's missing at the moment is that the prefix passed
to verify_filename should be modified by the one specified on the
command line.

> As for the patch itself, I haven't looked at it closely.  My only
> immediate reaction is that I wish it touched Documentation/ and t/. :)

I'll make sure the next version does.  This version was doing the
minimum required to make patch 2/2 possible, it certainly needs some
polish before it's more than a proof-of-concept.