Re: [PATCH v8 00/19] port branch.c to use ref-filter's printing options

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

On Thu, Dec 8, 2016 at 5:31 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>> index f4ad297..c72baeb 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -92,13 +92,14 @@ refname::
>>         The name of the ref (the part after $GIT_DIR/).
>>         For a non-ambiguous short name of the ref append `:short`.
>>         The option core.warnAmbiguousRefs is used to select the strict
>> -       abbreviation mode. If `strip=<N>` is appended, strips `<N>`
>> -       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. For the base
>> -       directory of the ref (i.e. foo in refs/foo/bar/boz) append
>> -       `:base`. For the entire directory path append `:dir`.
>> +       abbreviation mode. If `lstrip=<N>` or `rstrip=<N>` option can
>
> Grammar here, drop the If before `lstrip since you're referring to
> multiples and you say "x can be appended to y" rather than "if x is
> added, do y"
>

Will do. Thanks.

>> +       be appended to strip `<N>` slash-separated path components
>> +       from or end of the refname respectively (e.g.,
>> +       `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo` and
>> +       `%(refname:rstrip=2)` turns `refs/tags/foo` into `refs`).  if
>> +       `<N>` is a negative number, then only `<N>` path components
>> +       are left behind.  If a displayed ref has fewer components than
>> +       `<N>`, the command aborts with an error.
>>
>
> Would it make more sense to not die and instead just return the empty
> string? On the one hand, if we die() it's obvious that you tried to
> strip too many components. But on the other hand, it's also somewhat
> annoying to have the whole command fail because we happen upon a
> single ref that has fewer components?
>
> So, for positive numbers, we simply strip what we can, which may
> result in the empty string, and for negative numbers, we keep up to
> what we said, while potentially keeping the entire string. I feel
> that's a better alternative than a die() in the middle of a ref
> filter..
>
> What are other people's thoughts on this?

I am _for_ this. Even I think it'd be better to return an empty string rather
than just die in the middle.

-- 
Regards,
Karthik Nayak