Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms

[Karthik Nayak <karthik.188@gmail.com>]

On Mon, Nov 14, 2016 at 7:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>>>> index 600b703..f4ad297 100644
>>>> --- a/Documentation/git-for-each-ref.txt
>>>> +++ b/Documentation/git-for-each-ref.txt
>>>> @@ -96,7 +96,9 @@ refname::
>>>>         slash-separated path components from the front of the refname
>>>>         (e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
>>>>         `<N>` must be a positive integer.  If a displayed ref has fewer
>>>> -       components than `<N>`, the command aborts with an error.
>>>> +       components than `<N>`, the command aborts with an error. For the base
>>>> +       directory of the ref (i.e. foo in refs/foo/bar/boz) append
>>>> +       `:base`. For the entire directory path append `:dir`.
>
> Sorry that I missed this so far and I do not know how many recent
> rerolls had them like this, but I am not sure about these :base and
> :dir suffixes.  From their names I think readers would expect that
> they do rough equivalents to basename() and dirname() applied to the
> refname, but the example contradicts with that intuition.  The
> result of applying basename() to 'refs/boo/bar/boz' would be 'boz'
> and not 'foo' as the example says.
>

True that the options ':dir' and ':base' seem to be conflicting with
the use case
of basename() and dirname(), These were names which I thought best fit
the options
with no relation to basename() and dirname(). But now that you mention
it, it would
make sense to change these names to something more suitable, any suggestions
would be welcome.

> So assuming that :base and :dir are unrelated to basename() and
> dirname():
>
>  - I think calling these :base and :dir may be misleading
>
>  - More importantly, what do these do?  I do not think of a good
>    description that generalizes "base of refs/foo/bar/boz is foo" to
>    explain your :base.

$ ./git for-each-ref --format "%(refname)%(end) %(refname:dir)"
refs/heads/master                  refs/heads
refs/heads/ref-filter                refs/heads
refs/remotes/junio/va/i18n     refs/remotes/junio/va

$ ./git for-each-ref  refs/heads --format
"%(align:left,30)%(refname)%(end) %(refname:base)"
refs/heads/master                 heads
refs/heads/ref-filter                heads
refs/remotes/junio/va/i18n     remotes

I guess this should clear it up.

>
>  - A :dir that corresponds to the :base that picks 'foo' from
>    'refs/foo/bar/boz' needs an example, too.
>

I could add in an example.

> Or is the above example simply a typo?  Is refs/foo/bar/boz:base
> 'boz', not 'foo'?
>
>

Not a typo, but open to changes in name.

-- 
Regards,
Karthik Nayak