Re: [PATCH RFC] cherry: support --abbrev option

> Am 16.03.2010 00:50, schrieb Erik Faye-Lund:
>> It's still just nitpicking, and I appreciate the feed-back. I'm a
>> little bit hesitant here though, for the following reasons:
>> - All other users of OPT__ABBREV (with the exception of ls-files,
>> ls-tree and show-ref) initialize abbrev to it's default value (but
>> they all use DEFAULT_ABBREV).
>> - ls-files and ls-tree (but not show-ref) both does the following,
>> when using abbrev: "abbrev ? find_unique_abbrev(ce->sha1,abbrev) :
>> sha1_to_hex(ce->sha1)".
>> - ls-files, ls-tree and show-ref, all seems to default "abbrev" to
>> zero (by making it a static global).
>> - I want to be consistent with the existing code.
>>
>> So, basically, ls-files and ls-tree seems to think
>> find_unique_abbrev() does not correctly (for this purpose) handle
>> abbrev=0. However, show-ref does seem to assume so. Looking at the
>> implementation of find_unique_abbrev(), it is clear that it does. But
>> as I said: I want to be consistent, and the variation from show-ref
>> (basically what you're suggesting) is the least common one.
>>
>> So I guess I can either:
>> 1) Change the code to be consistent with show-ref, and submit an
>> additional patch to make ls-files and ls-tree consistent with this.
>> This might have a performance-impact though, since
>> find_unique_abbrev() does some extra work (checking the sha1 for
>> existence and an extra buffer-copy).
>> 2) Change the code as you suggest, and not care so much about consistency.
>> 3) Leave the code to be functionally consistent with those who
>> initialize abbrev to DEFAULT_ABBREV (but with a different default,
>> which in itself is slightly inconsistent).
>>
>> I'm leaning towards 3) for now, but I don't have any strong feelings.
>
> If you do 2) then it stands 2:2 (ls-files, ls-tree vs. show-ref, cherry). :)
>