From: Jacob Keller <jacob.keller@gmail.com>
Date: Fri, 11 Nov 2016 05:29:13 GMT
Subject: Re: [PATCH v7 04/17] ref-filter: modify "%(objectname:short)" to take length
Message-ID: <CA+P7+xqXY6XvUm8Kna_GwY4XOSSGPM=-ezZECgkGcbnX_Penmg@mail.gmail.com>
In-Reply-To: <CAOLa=ZRDNGGc-OA+d+T1=9JvoVe2eBZt0DYQEHmeNKhDpxQbdg@mail.gmail.com>

On Thu, Nov 10, 2016 at 9:36 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
> On Wed, Nov 9, 2016 at 4:57 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Tue, Nov 8, 2016 at 12:11 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>>> From: Karthik Nayak <karthik.188@gmail.com>
>>>
>>> Add support for %(objectname:short=<length>) which would print the
>>> abbreviated unique objectname of given length. When no length is
>>> specified, the length is 'DEFAULT_ABBREV'. The minimum length is
>>> 'MINIMUM_ABBREV'. The length may be exceeded to ensure that the provided
>>> object name is unique.
>>>
>>
>> Ok this makes sense. It may be annoying that the length might go
>> beyond the size that we wanted, but I think it's better than printing
>> a non-unique short abbreviation.
>>
>> I have one suggested change, which is to drop O_LENGTH and have
>> O_SHORT store the length always, setting it to DEFAULT_ABBREV when no
>> length provided. This allows you to drop some code. I don't think it's
>> actually worth a re-roll by itself since the current code is correct.
>>
>> Thanks,
>> Jake
>>
>
> That does make sense, It would also not error out when we use
> %(objectname:short=) and
> not specify the length. Idk, if that's desirable or not. But it does
> make the code a little more
> confusing to read at the same time.
>

I am not sure that would be the case. If you see "objectname:short"
you trreat this as if they had passed "objectname:short=<default
abbrev>" but if you see "objectname:short=" you die, no?

> So since its a small change, I'd be okay going either ways with this.
>
> --
> Regards,
> Karthik Nayak