Re: [PATCH v7 04/17] ref-filter: modify "%(objectname:short)" to take length

From: Karthik Nayak <karthik.188@gmail.com>
Date: Nov 10, 2016, 17:36 UTC
Message-ID: <CAOLa=ZRDNGGc-OA+d+T1=9JvoVe2eBZt0DYQEHmeNKhDpxQbdg@mail.gmail.com>
In-Reply-To: <CA+P7+xqG5ACT2ZB=M8gLmDaYgSgX202+afJmZJKat6NzbFzL=g@mail.gmail.com>

On Wed, Nov 9, 2016 at 4:57 AM, Jacob Keller <jacob.keller@gmail.com> wrote:

Show 22 quoted lines

> 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.

So since its a small change, I'd be okay going either ways with this.

-- 
Regards,
Karthik Nayak