From: Karthik Nayak Date: Wed, 29 Jul 2015 17:54:49 GMT Subject: Re: [RFC/PATCH 06/11] branch: roll show_detached HEAD into regular ref_list Message-ID: In-Reply-To: On Wed, Jul 29, 2015 at 3:26 PM, Matthieu Moy wrote: > Karthik Nayak writes: > >> On Tue, Jul 28, 2015 at 6:31 PM, Matthieu Moy >> 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