Re: [PATCH v5 7/8] branch.c: use 'ref-filter' APIs
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Sep 20, 2015, 19:24 UTC
- Message-ID
- <vpqmvwgevx3.fsf@anie.imag.fr>
- In-Reply-To
- <1442772627-25421-8-git-send-email-Karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 13 quoted lines
> --- 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.
Show 11 quoted lines
> -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).
-- Matthieu Moy http://www-verimag.imag.fr/~moy/