Re: [PATCH v5 7/8] branch.c: use 'ref-filter' APIs

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> --- a/Documentation/git-branch.txt
>> +++ b/Documentation/git-branch.txt
>> @@ -231,6 +231,13 @@ start-point is either a local or remote-tracking branch.
>>       The new name for an existing branch. The same restrictions as for
>>       <branchname> apply.
>>
>> +--sort=<key>::
>> +     Sort based on the key given. Prefix `-` to sort in descending
>> +     order of the value. You may use the --sort=<key> option
>> +     multiple times, in which case the last key becomes the primary
>> +     key. The keys supported are the same as those in `git
>> +     for-each-ref`. Sort order defaults to sorting based on branch
>> +     type.
>
> The last sentence is no longer true. Perhaps something like:
>
> Sort order defaults to sorting based on the full refname (including
> `refs/...` prefix). This lists detached HEAD (if present) first, then
> local branches and finally remote-tracking branches.
>

>> -static void print_ref_list(struct ref_filter *filter)
>> +static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting)
>>  {
>>       int i;
>>       struct ref_array array;
>> -     struct ref_filter_cbdata data;
>>       int maxwidth = 0;
>>       const char *remote_prefix = "";
>> -     struct rev_info revs;
>> +     struct ref_sorting def_sorting;
>> +     const char *sort_type = "refname";
>
> You are using refname without special-casing HEAD at all. So, this is
> relying on the fact that HEAD comes alphabetically before refs/..., and
> that we are only listing refs/... and HEAD.
>
> As I mentionned earlyer, if we ever allow branch to list e.g.
> FETCH_HEAD, then the detached HEAD will come in the middle. I first
> thought that this was too fragile, but after thinking about it, I think
> this is actually sensible. After all, if we ever allow FETCH_HEAD, then
> keeping the alphabetical order still makes sense.
>
> So, your code is OK, but a bit too subtle to my taste: you should add a
> comment explaining why sorting according to refname is sufficient (I
> would use a comment, but a better commit message may be OK too).