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

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

On Fri, Nov 18, 2016 at 11:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
>>>>> to get remotes from /refs/foo/abc/xyz we'd need to do
>>>>> strip=1,strip=-1, which could be
>>>>> done but ...
>>>>
>>>> ... would be unnecessary if this is the only use case:
>>>>
>>>>> strbuf_addf(&fmt,
>>>>> "%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)",
>>>>> local.buf, remote.buf);
>>>>
>>>> You can "strip to leave only 2 components" and compare the result
>>>> with refs/remotes instead, no?
>>>>
>>>
>>> Of course, my only objective was that someone would find it useful to
>>> have these two additional
>>> atoms. So if you think it's unnecessary we could drop it entirely :D
>>>
>>> --
>>> Regards,
>>> Karthik Nayak
>>
>> I think having strip and rstrip make sense, (along with support for
>> negative numbers) I don't think we need to make them work together
>> unless someone is interested, since we can use strip=-2 to get the
>> behavior we need today.
>
> I am OK with multiple strips Karthik suggests, e.g.
>
>     %(refname:strip=1,rstrip=-1)
>
> if it is cleanly implemented.
>
> I have a bit of trouble with these names, though.  If we call one
> strip and the other rstrip, to only those who know about rstrip it
> would be clear that strip is about stripping from the left.  Perhaps
> we should call it lstrip for symmetry and ease-of-remembering?
>
>     refs/heads/master:lstrip=-1 => master (strip all but one level
>     from the left)
>
>     refs/heads/master:rstrip=-2 => refs/heads (strip all but two
>     levels from the right)
>
>     refs/heads/master:lstrip=1,rstrip=-1 => heads (strip one level
>     from the left and then strip all but one level from the right)
>
> I dunno.

We could have lstrip and rstrip as you suggested and perhaps make it work
together too. But I see this going off the scope of this series. Maybe
I'll follow up
with another series introducing these features. Since we can currently
make do with
'strip=2' I'll drop this patch from v8 of this series and pursue this
idea after this.

On Sat, Nov 19, 2016 at 3:19 AM, Jakub Narębski <jnareb@gmail.com> wrote:
> W dniu 15.11.2016 o 18:42, Junio C Hamano pisze:
>> Jacob Keller <jacob.keller@gmail.com> writes:
>>
>>> dirname makes sense. What about implementing a reverse variant of
>>> strip, which you could perform stripping of right-most components and
>>> instead of stripping by a number, strip "to" a number, ie: keep the
>>> left N most components, and then you could use something like
>>> ...
>>> I think that would be more general purpose than basename, and less confusing?
>>
>> I think you are going in the right direction.  I had a similar
>> thought but built around a different axis.  I.e. if strip=1 strips
>> one from the left, perhaps we want to have rstrip=1 that strips one
>> from the right, and also strip=-1 to mean strip everything except
>> one from the left and so on?.  I think this and your keep (and
>> perhaps you'll have rkeep for completeness) have the same expressive
>> power.  I do not offhand have a preference one over the other.
>>
>> Somehow it sounds a bit strange to me to treat 'remotes' as the same
>> class of token as 'heads' and 'tags' (I'd expect 'heads' and
>> 'remotes/origin' would be at the same level in end-user's mind), but
>> that is probably an unrelated tangent.  The reason this series wants
>> to introduce :base must be to emulate an existing feature, so that
>> existing feature is a concrete counter-example that argues against
>> my "it sounds a bit strange" reaction.
>
> If it is to implement the feature where we select if to display only
> local branches (refs/heads/**), only remote-tracking branches
> (refs/remotes/**/**), or only tags (refs/tags/**), then perhaps
> ':category' or ':type' would make sense?
>
> As in '%(refname:category)', e.g.
>
>   %(if:equals=heads)%(refname:category)%(then)...%(end)
>

This is also a good idea but would bring about the same confusion that Junio
was referring to, i.e.

"Somehow it sounds a bit strange to me to treat 'remotes' as the same
class of token as 'heads' and 'tags' (I'd expect 'heads' and
'remotes/origin' would be at the same level in end-user's mind), but
that is probably an unrelated tangent.  The reason this series wants
to introduce :base must be to emulate an existing feature, so that
existing feature is a concrete counter-example that argues against
my "it sounds a bit strange" reaction."

So right now the rstrip/lstrip idea seems to be a good way to go about
this, but I
think that'd be after this series.

-- 
Regards,
Karthik Nayak