git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matthew DeVore <matvore@google.com>
To: git@vger.kernel.org
Cc: avarab@gmail.com, Johannes.Schindelin@gmx.de,
	git@matthieu-moy.fr, olyatelezhnaya@gmail.com,
	samuel.maftoul@gmail.com, gitster@pobox.com,
	karthik.188@gmail.com, pclouds@gmail.com,
	sunshine@sunshineco.com, emilyshaffer@google.com, jrn@google.com,
	matvore@google.com
Subject: [RFC PATCH] ref-filter: sort detached HEAD lines firstly
Date: Thu, 6 Jun 2019 14:38:20 -0700	[thread overview]
Message-ID: <faaa9a3d6ba66d77cc2a8eab438d1bfc8f762fa1.1559857032.git.matvore@google.com> (raw)

Before this patch, "git branch" would put "(HEAD detached...)" and "(no
branch, rebasing...)" lines before all the other branches *in most
cases* and only because of the fact that "(" is a low codepoint. This
would not hold in the Chinese locale, which uses a full-width "(" symbol
(codepoint FF08). This meant that the detached HEAD line would appear
after all local refs and even after the remote refs if there were any.

Deliberately sort the detached HEAD refs before other refs when sorting
by refname rather than rely on codepoint subtleties.

Signed-off-by: Matthew DeVore <matvore@google.com>
---
 ref-filter.c           | 10 +++++++---
 t/lib-gettext.sh       | 16 +++++++++++++---
 t/t3207-branch-intl.sh | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 6 deletions(-)
 create mode 100755 t/t3207-branch-intl.sh

diff --git a/ref-filter.c b/ref-filter.c
index 8500671bc6..cbfae790f9 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2157,25 +2157,29 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
 	cmp_type cmp_type = used_atom[s->atom].type;
 	int (*cmp_fn)(const char *, const char *);
 	struct strbuf err = STRBUF_INIT;
 
 	if (get_ref_atom_value(a, s->atom, &va, &err))
 		die("%s", err.buf);
 	if (get_ref_atom_value(b, s->atom, &vb, &err))
 		die("%s", err.buf);
 	strbuf_release(&err);
 	cmp_fn = s->ignore_case ? strcasecmp : strcmp;
-	if (s->version)
+	if (s->version) {
 		cmp = versioncmp(va->s, vb->s);
-	else if (cmp_type == FIELD_STR)
+	} else if (cmp_type == FIELD_STR) {
+		if ((a->kind & FILTER_REFS_DETACHED_HEAD) !=
+				(b->kind & FILTER_REFS_DETACHED_HEAD)) {
+			return (a->kind & FILTER_REFS_DETACHED_HEAD) ? -1 : 1;
+		}
 		cmp = cmp_fn(va->s, vb->s);
-	else {
+	} else {
 		if (va->value < vb->value)
 			cmp = -1;
 		else if (va->value == vb->value)
 			cmp = cmp_fn(a->refname, b->refname);
 		else
 			cmp = 1;
 	}
 
 	return (s->reverse) ? -cmp : cmp;
 }
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 2139b427ca..de08d109dc 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -25,23 +25,29 @@ then
 		p
 		q
 	}')
 	# is_IS.ISO8859-1 on Solaris and FreeBSD, is_IS.iso88591 on Debian
 	is_IS_iso_locale=$(locale -a 2>/dev/null |
 		sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
 		p
 		q
 	}')
 
-	# Export them as an environment variable so the t0202/test.pl Perl
-	# test can use it too
-	export is_IS_locale is_IS_iso_locale
+	zh_CN_locale=$(locale -a 2>/dev/null |
+		sed -n '/^zh_CN\.[uU][tT][fF]-*8$/{
+		p
+		q
+	}')
+
+	# Export them as environment variables so other tests can use them
+	# too
+	export is_IS_locale is_IS_iso_locale zh_CN_locale
 
 	if test -n "$is_IS_locale" &&
 		test $GIT_INTERNAL_GETTEXT_SH_SCHEME != "fallthrough"
 	then
 		# Some of the tests need the reference Icelandic locale
 		test_set_prereq GETTEXT_LOCALE
 
 		# Exporting for t0202/test.pl
 		GETTEXT_LOCALE=1
 		export GETTEXT_LOCALE
@@ -53,11 +59,15 @@ then
 	if test -n "$is_IS_iso_locale" &&
 		test $GIT_INTERNAL_GETTEXT_SH_SCHEME != "fallthrough"
 	then
 		# Some of the tests need the reference Icelandic locale
 		test_set_prereq GETTEXT_ISO_LOCALE
 
 		say "# lib-gettext: Found '$is_IS_iso_locale' as an is_IS ISO-8859-1 locale"
 	else
 		say "# lib-gettext: No is_IS ISO-8859-1 locale available"
 	fi
+
+	if test -z "$zh_CN_locale"; then
+		say "# lib-gettext: No zh_CN UTF-8 locale available"
+	fi
 fi
diff --git a/t/t3207-branch-intl.sh b/t/t3207-branch-intl.sh
new file mode 100755
index 0000000000..9f6fcc7481
--- /dev/null
+++ b/t/t3207-branch-intl.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='git branch internationalization tests'
+
+. ./lib-gettext.sh
+
+test_expect_success 'init repo' '
+	git init r1 &&
+	touch r1/foo &&
+	git -C r1 add foo &&
+	git -C r1 commit -m foo
+'
+
+test_expect_success 'detached head sorts before other branches' '
+	# Ref sorting logic should put detached heads before the other
+	# branches, but this is not automatic when a branch name sorts
+	# lexically before "(" or the full-width "(" (Unicode codepoint FF08).
+	# The latter case is nearly guaranteed for the Chinese locale.
+
+	git -C r1 checkout HEAD^{} -- &&
+	git -C r1 branch !should_be_after_detached HEAD &&
+	LC_ALL=$zh_CN_locale LC_MESSAGES=$zh_CN_locale \
+		git -C r1 branch >actual &&
+	git -C r1 checkout - &&
+
+	awk "
+	# We need full-width or half-width parens on the first line.
+	NR == 1 && (/[(].*[)]/ || /\xef\xbc\x88.*\xef\xbc\x89/) {
+		found_head = 1;
+	}
+	/!should_be_after_detached/ {
+		found_control_branch = 1;
+	}
+	END { exit !found_head || !found_control_branch }
+	" actual
+'
+
+test_done
-- 
2.21.0


             reply	other threads:[~2019-06-06 21:39 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-06 21:38 Matthew DeVore [this message]
2019-06-09  8:17 ` [RFC PATCH] ref-filter: sort detached HEAD lines firstlyxy Johannes Schindelin
2019-06-09 16:39   ` Johannes Schindelin
2019-06-10 23:49   ` [RFC PATCH] ref-filter: sort detached HEAD lines firstly Matthew DeVore
2019-06-11  0:41     ` Jonathan Nieder
2019-06-11 16:48       ` Matthew DeVore
2019-06-11 19:53       ` Junio C Hamano
2019-06-11 18:28 ` [PATCH v2 0/1] Sort " Matthew DeVore
2019-06-11 18:28   ` [PATCH v2 1/1] ref-filter: sort " Matthew DeVore
2019-06-11 20:10     ` Junio C Hamano
2019-06-12 21:09       ` Junio C Hamano
2019-06-12 21:21         ` Matthew DeVore
2019-06-13  1:56         ` Matthew DeVore
2019-06-12 19:51     ` Johannes Schindelin
2019-06-13 16:58       ` Jeff King
2019-06-18 22:29 ` [PATCH v3 0/1] Sort detached heads line firstly Matthew DeVore
2019-06-18 22:29   ` [PATCH v3 1/1] ref-filter: sort detached HEAD lines firstly Matthew DeVore
2019-06-19 15:29     ` Junio C Hamano
2021-01-06 10:01     ` [PATCH 0/5] branch: --sort improvements Ævar Arnfjörð Bjarmason
2021-01-07  9:51       ` [PATCH v2 0/7] " Ævar Arnfjörð Bjarmason
2021-01-07  9:51       ` [PATCH v2 1/7] branch: change "--local" to "--list" in comment Ævar Arnfjörð Bjarmason
2021-01-07  9:51       ` [PATCH v2 2/7] branch tests: add to --sort tests Ævar Arnfjörð Bjarmason
2021-01-07  9:51       ` [PATCH v2 3/7] ref-filter: add braces to if/else if/else chain Ævar Arnfjörð Bjarmason
2021-01-07  9:51       ` [PATCH v2 4/7] ref-filter: move "cmp_fn" assignment into "else if" arm Ævar Arnfjörð Bjarmason
2021-01-07  9:51       ` [PATCH v2 5/7] ref-filter: move ref_sorting flags to a bitfield Ævar Arnfjörð Bjarmason
2021-01-07 23:24         ` Junio C Hamano
2021-01-07  9:51       ` [PATCH v2 6/7] branch: sort detached HEAD based on a flag Ævar Arnfjörð Bjarmason
2021-01-07  9:51       ` [PATCH v2 7/7] branch: show "HEAD detached" first under reverse sort Ævar Arnfjörð Bjarmason
2021-01-06 10:01     ` [PATCH 1/5] branch: change "--local" to "--list" in comment Ævar Arnfjörð Bjarmason
2021-01-06 10:01     ` [PATCH 2/5] branch tests: add to --sort tests Ævar Arnfjörð Bjarmason
2021-01-06 23:21       ` Junio C Hamano
2021-01-06 10:01     ` [PATCH 3/5] ref-filter: add a "detached_head_first" sorting option Ævar Arnfjörð Bjarmason
2021-01-06 23:45       ` Junio C Hamano
2021-01-06 10:01     ` [PATCH 4/5] branch: use the " Ævar Arnfjörð Bjarmason
2021-01-06 10:01     ` [PATCH 5/5] branch: show "HEAD detached" first under reverse sort Ævar Arnfjörð Bjarmason
2021-01-06 23:49       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=faaa9a3d6ba66d77cc2a8eab438d1bfc8f762fa1.1559857032.git.matvore@google.com \
    --to=matvore@google.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@matthieu-moy.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrn@google.com \
    --cc=karthik.188@gmail.com \
    --cc=olyatelezhnaya@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=samuel.maftoul@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).