Addresses a comment Junio had on 3/5 in v1. Now there's leading patches moving the ref_sorting flags to a bitfield, which (agreeing with Junio) I think looks a bit better & should be more maintainable going forward. While I was at it I squashed the 3/5 and 4/5 patches into one. It's less confusing to look at when we add the ref-filter.c sorting code in the same commit we're using it in. I also made the bitfield/xor checking/sanity BUG a bit less verbose & simpler to understand. Ævar Arnfjörð Bjarmason (7): branch: change "--local" to "--list" in comment branch tests: add to --sort tests ref-filter: add braces to if/else if/else chain ref-filter: move "cmp_fn" assignment into "else if" arm ref-filter: move ref_sorting flags to a bitfield branch: sort detached HEAD based on a flag branch: show "HEAD detached" first under reverse sort builtin/branch.c | 6 ++-- builtin/for-each-ref.c | 2 +- builtin/tag.c | 2 +- ref-filter.c | 75 ++++++++++++++++++++++++---------------- ref-filter.h | 13 ++++--- t/t3203-branch-output.sh | 51 ++++++++++++++++++++++++++- wt-status.c | 4 +-- wt-status.h | 2 -- 8 files changed, 111 insertions(+), 44 deletions(-) Range-diff: 1: c74e75dea90 = 1: c74e75dea90 branch: change "--local" to "--list" in comment 2: 1fea125c7a6 = 2: 1fea125c7a6 branch tests: add to --sort tests 3: 11e6f274d2d < -: ----------- ref-filter: add a "detached_head_first" sorting option -: ----------- > 3: 5cb44f0be40 ref-filter: add braces to if/else if/else chain -: ----------- > 4: 3e26cebe545 ref-filter: move "cmp_fn" assignment into "else if" arm -: ----------- > 5: ad598fdc87c ref-filter: move ref_sorting flags to a bitfield 4: faf9e23a13f ! 6: af0c884b506 branch: use the "detached_head_first" sorting option @@ Metadata Author: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> ## Commit message ## - branch: use the "detached_head_first" sorting option + branch: sort detached HEAD based on a flag - Use the new ref_sorting_detached_head_first_all() sorting option in - ref-filter.c to revert and amend 28438e84e04 (ref-filter: sort - detached HEAD lines firstly, 2019-06-18). + Change the ref-filter sorting of detached HEAD to check the + FILTER_REFS_DETACHED_HEAD flag, instead of relying on the ref + description filled-in by get_head_description() to start with "(", + which in turn we expect to ASCII-sort before any other reference. - In Chinese the fullwidth versions of punctuation like "()" are - typically written as (U+FF08 fullwidth left parenthesis), (U+FF09 - fullwidth right parenthesis) instead. This form is used in both - po/zh_{CN,TW}.po in most cases where "()" is translated in a string. - - In 28438e84e04 the ability to translate this as part of the "git - branch -l" output was removed because we'd like the detached line to - appear first at the start of "git branch -l", e.g.: + For context, we'd like the detached line to appear first at the start + of "git branch -l", e.g.: $ git branch -l * (HEAD detached at <hash>) master - Let's instead use the new ref_sorting_detached_head_first_all() in - branch.c to say that we'd like these sorted before other entries. + This doesn't change that, but improves on a fix made in + 28438e84e04 (ref-filter: sort detached HEAD lines firstly, 2019-06-18) + and gives the Chinese translation the ability to use its preferred + punctuation marks again. + + In Chinese the fullwidth versions of punctuation like "()" are + typically written as (U+FF08 fullwidth left parenthesis), (U+FF09 + fullwidth right parenthesis) instead[1]. This form is used in both + po/zh_{CN,TW}.po in most cases where "()" is translated in a string. + + Aside from that improvement to the Chinese translation, it also just + makes for cleaner code that we mark any special cases in the ref_array + we're sorting with flags and make the sort function aware of them, + instead of piggy-backing on the general-case of strcmp() doing the + right thing. As seen in the amended tests this made reverse sorting a bit more consistent. Before this we'd sometimes sort this message in the - middle, now it's consistently at the beginning or end. Having it at - the end doesn't make much sense either, but at least it behaves - consistently now. A follow-up commit will make this behavior even - better. + middle, now it's consistently at the beginning or end, depending on + whether we're doing a normal or reverse sort. Having it at the end + doesn't make much sense either, but at least it behaves consistently + now. A follow-up commit will make this behavior under reverse sorting + even better. I'm removing the "TRANSLATORS" comments that were in the old code while I'm at it. Those were added in d4919bb288e (ref-filter: move @@ builtin/branch.c @@ builtin/branch.c: int cmd_branch(int argc, const char **argv, const char *prefix) if (!sorting) sorting = ref_default_sorting(); - ref_sorting_icase_all(sorting, icase); -+ ref_sorting_detached_head_first_all(sorting, 1); + ref_sorting_set_sort_flags_all(sorting, REF_SORTING_ICASE, icase); ++ ref_sorting_set_sort_flags_all( ++ sorting, REF_SORTING_DETACHED_HEAD_FIRST, 1); print_ref_list(&filter, sorting, &format); print_columns(&output, colopts, NULL); string_list_clear(&output, 0); @@ ref-filter.c: char *get_head_description(void) return strbuf_detach(&desc, NULL); } +@@ ref-filter.c: int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int + return ret; + } + ++static int compare_detached_head(struct ref_array_item *a, struct ref_array_item *b) ++{ ++ if (!(a->kind ^ b->kind)) ++ BUG("ref_kind_from_refname() should only mark one ref as HEAD"); ++ if (a->kind & FILTER_REFS_DETACHED_HEAD) ++ return -1; ++ else if (b->kind & FILTER_REFS_DETACHED_HEAD) ++ return 1; ++ BUG("should have died in the xor check above"); ++ return 0; ++} ++ + static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b) + { + struct atom_value *va, *vb; +@@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru + if (get_ref_atom_value(b, s->atom, &vb, &err)) + die("%s", err.buf); + strbuf_release(&err); +- if (s->sort_flags & REF_SORTING_VERSION) { ++ if (s->sort_flags & REF_SORTING_DETACHED_HEAD_FIRST && ++ ((a->kind | b->kind) & FILTER_REFS_DETACHED_HEAD)) { ++ cmp = compare_detached_head(a, b); ++ } else if (s->sort_flags & REF_SORTING_VERSION) { + cmp = versioncmp(va->s, vb->s); + } else if (cmp_type == FIELD_STR) { + int (*cmp_fn)(const char *, const char *); + + ## ref-filter.h ## +@@ ref-filter.h: struct ref_sorting { + REF_SORTING_REVERSE = 1<<0, + REF_SORTING_ICASE = 1<<1, + REF_SORTING_VERSION = 1<<2, ++ REF_SORTING_DETACHED_HEAD_FIRST = 1<<3, + } sort_flags; + }; + ## t/t3203-branch-output.sh ## @@ t/t3203-branch-output.sh: test_expect_success 'git branch `--sort=[-]objectsize` option' ' 5: b14a7b32cbf ! 7: 2497fffebf6 branch: show "HEAD detached" first under reverse sort @@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array int cmp; + int cmp_detached_head = 0; cmp_type cmp_type = used_atom[s->atom].type; - int (*cmp_fn)(const char *, const char *); struct strbuf err = STRBUF_INIT; + @@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru - ^ - (b->kind & FILTER_REFS_DETACHED_HEAD))) { + if (s->sort_flags & REF_SORTING_DETACHED_HEAD_FIRST && + ((a->kind | b->kind) & FILTER_REFS_DETACHED_HEAD)) { cmp = compare_detached_head(a, b); + cmp_detached_head = 1; - } else if (s->version) + } else if (s->sort_flags & REF_SORTING_VERSION) { cmp = versioncmp(va->s, vb->s); - else if (cmp_type == FIELD_STR) + } else if (cmp_type == FIELD_STR) { @@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru cmp = 1; } -- return (s->reverse) ? -cmp : cmp; -+ return (s->reverse && !cmp_detached_head) ? -cmp : cmp; +- return (s->sort_flags & REF_SORTING_REVERSE) ? -cmp : cmp; ++ return (s->sort_flags & REF_SORTING_REVERSE && !cmp_detached_head) ++ ? -cmp : cmp; } static int compare_refs(const void *a_, const void *b_, void *ref_sorting) -- 2.29.2.222.g5d2a92d10f8