git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/7] branch: --sort improvements

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jan 7, 2021, 09:51 UTC
Message-ID
<20210107095153.4753-1-avarab@gmail.com>
In-Reply-To
<20210106100139.14651-1-avarab@gmail.com>

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@gmail.com>
     
      ## 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
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 21 of 36 in “ref-filter: sort detached HEAD lines firstly”
  1. ref-filter: sort detached HEAD lines firstlyMatthew DeVore, Jun 6, 2019
  2. Johannes SchindelinJun 9, 2019
  3. Johannes SchindelinJun 9, 2019
  4. Matthew DeVoreJun 10, 2019
  5. Jonathan NiederJun 11, 2019
  6. Matthew DeVoreJun 11, 2019
  7. Junio C HamanoJun 11, 2019
  8. 0/1 Sort detached HEAD lines firstlyMatthew DeVore, Jun 11, 2019
  9. 1/1 ref-filter: sort detached HEAD lines firstlyMatthew DeVore, Jun 11, 2019
  10. Junio C HamanoJun 11, 2019
  11. Junio C HamanoJun 12, 2019
  12. Matthew DeVoreJun 13, 2019
  13. Matthew DeVoreJun 12, 2019
  14. Johannes SchindelinJun 12, 2019
  15. Jeff KingJun 13, 2019
  16. 0/1 Sort detached heads line firstlyMatthew DeVore, Jun 18, 2019
  17. 1/1 ref-filter: sort detached HEAD lines firstlyMatthew DeVore, Jun 18, 2019
  18. Junio C HamanoJun 19, 2019
  19. 0/5 branch: --sort improvementsÆvar Arnfjörð Bjarmason, Jan 6, 2021
  20. 1/7 branch: change "--local" to "--list" in commentÆvar Arnfjörð Bjarmason, Jan 7, 2021
  21. 0/7 branch: --sort improvementsÆvar Arnfjörð Bjarmason, Jan 7, 2021
  22. 4/7 ref-filter: move "cmp_fn" assignment into "else if" armÆvar Arnfjörð Bjarmason, Jan 7, 2021
  23. 2/7 branch tests: add to --sort testsÆvar Arnfjörð Bjarmason, Jan 7, 2021
  24. 3/7 ref-filter: add braces to if/else if/else chainÆvar Arnfjörð Bjarmason, Jan 7, 2021
  25. 6/7 branch: sort detached HEAD based on a flagÆvar Arnfjörð Bjarmason, Jan 7, 2021
  26. 7/7 branch: show "HEAD detached" first under reverse sortÆvar Arnfjörð Bjarmason, Jan 7, 2021
  27. 5/7 ref-filter: move ref_sorting flags to a bitfieldÆvar Arnfjörð Bjarmason, Jan 7, 2021
  28. Junio C HamanoJan 7, 2021
  29. 1/5 branch: change "--local" to "--list" in commentÆvar Arnfjörð Bjarmason, Jan 6, 2021
  30. 2/5 branch tests: add to --sort testsÆvar Arnfjörð Bjarmason, Jan 6, 2021
  31. Junio C HamanoJan 6, 2021
  32. 4/5 branch: use the "detached_head_first" sorting optionÆvar Arnfjörð Bjarmason, Jan 6, 2021
  33. 3/5 ref-filter: add a "detached_head_first" sorting optionÆvar Arnfjörð Bjarmason, Jan 6, 2021
  34. Junio C HamanoJan 6, 2021
  35. 5/5 branch: show "HEAD detached" first under reverse sortÆvar Arnfjörð Bjarmason, Jan 6, 2021
  36. Junio C HamanoJan 6, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.