Re: [RFC/PATCH 06/11] branch: roll show_detached HEAD into regular ref_list

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

On Wed, Jul 29, 2015 at 3:26 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> On Tue, Jul 28, 2015 at 6:31 PM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>>
>>>> -static void show_detached(struct ref_list *ref_list, int maxwidth)
>>>> -{
>>>> -     struct commit *head_commit = lookup_commit_reference_gently(head_sha1, 1);
>>>> -
>>>> -     if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
>>>
>>> I'm not sure what this if was doing, and why you can get rid of it. My
>>> understanding is that with_commit comes from --contains, and in the
>>> previous code the filtering was done at display time (detached HEAD was
>>> not shown if it was not contained in commits specified with --contains).
>>>
>>> Eventually, you'll use ref-filter to do this filtering so you won't need
>>> this check at display time.
>>>
>>> But am I correct that for a few commits, you ignore --contains on
>>> detached HEAD?
>>>
>>
>> No we don't ignore --contains on detached HEAD.
>>
>> Since detached HEAD now gets its data from append_ref(). The function
>> also checks for the --contains option.
>
> Ah, OK. Previously, detached HEAD and branches were completely
> different, each having its own if (is_descendant_of(...)), and you're
> now using only one in append_ref() before removing it completely in
> favor of ref-filter.
>
> That would deserve an explanation for other reviewers I think.
>

Will include a small explanation in the commit message :)

-- 
Regards,
Karthik Nayak