Re: [PATCH v4 3/8] branch: roll show_detached HEAD into regular ref_list

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Sep 17, 2015, 15:43 UTC
Message-ID: <vpqio79oxva.fsf@anie.imag.fr>
In-Reply-To: <xmqqh9mtkrg0.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 20 quoted lines

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>> So either we could introduce a new atom for sorting something like
>>> `branch_sort` which uses the FILTER_REFS_(DETACHED_HEAD | BRANCHES |
>>> REMOTES)
>>
>> I don't think you need a new atom. You can just change the sorting
>> function to consider that detached HEAD is always first, and when
>> comparing two non-detached-HEAD branches, use the atom supplied by the
>> user.
>>
>> That would mean the detached HEAD would be displayed first regardless of
>> --sort (which is the case right now).
>
> I am a bit fuzzy about this.  I do not understand why Karthik thinks
> a new atom is necessary in the first place, and I do agree that the
> best way to go would be to teach the sort function to do "the right
> thing", but I am not sure why it has to be "regardless of --sort".

I think Karthik meant that branch could default to "--sort=my_magic_atom_that_does_the_right_thing". In this case, the default would be to show HEAD first, but using "--sort" explicitly would change the order and interleave HEAD within other branches.

IOW, we have:

struct ref_sorting *ref_default_sorting(void)
{
	static const char cstr_name[] = "refname";

	struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));

	sorting->next = NULL;
	sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
	return sorting;
}

and we could

- static const char cstr_name[] = "refname"; + static const char cstr_name[] = "some_magic_atom";

But you convinced me that this is not a good idea.

> When the user does give a custom --sort criteria, the logic in
> default_sort()

The logic itself is not in ref_default_sorting() (I guess this is what you meant by "default_sort"): this function just builds a struct ref_sorting that is later used by the more general cmp_ref_sorting.

But that's still workable: struct ref_sorting could contain a flag "head_first" that would be set by ref_default_sorting() and only it, and then read by cmp_ref_sorting.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/