git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Pratyush Yadav <me@yadavpratyush.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>, 加藤一博 <kato-k@ksysllc.co.jp>
Subject: Re: [PATCH] git gui: fix branch name encoding error on git gui
Date: Mon, 09 Dec 2019 11:15:55 -0800	[thread overview]
Message-ID: <xmqqpngxux78.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20191207002842.32208-1-kato-k@ksysllc.co.jp> (=?utf-8?B?Ig==?= =?utf-8?B?5Yqg6Jek5LiA5Y2aIidz?= message of "Sat, 7 Dec 2019 00:29:09 +0000")

加藤一博 <kato-k@ksysllc.co.jp> writes:

> After "git checkout -b '漢字'" to create a branch with UTF-8
> character in it, "git gui" shows the branch name incorrectly,
> as it forgets to turn the bytes
> read from the "git for-each-ref" and
> read from "HEAD" file
> into Unicode characters.

Thanks.

Note to the git-gui mentainer.  The above may want to be
line-wrapped a bit.

> Signed-off-by: Kazuhiro Kato <kato-k@ksysllc.co.jp>
> ---
>  git-gui.sh     | 1 +
>  lib/branch.tcl | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 0d21f56..8f4a9ae 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -684,6 +684,7 @@ proc load_current_branch {} {
>  	global current_branch is_detached
>  
>  	set fd [open [gitdir HEAD] r]
> +	fconfigure $fd -translation binary -encoding utf-8
>  	if {[gets $fd ref] < 1} {
>  		set ref {}
>  	}

A comment totally outside the scope of this fix to anybody
interested in further working on this code.

This piece of code is way too intimate with the implementation
details of HEAD and yet not intimate enough to know that HEAD can be
a symlink (in other words, it is a poor imitation of the real logic
implemented in git core).  A kosher way to implement this would be
to call

	git symbolic-ref --quiet --short HEAD

which would succeed and give the branch name to its standard output,
or would fail when the head is detached.  Set "current_branch" and
"is_detached" according to the outcome.

And yes, Kato-san's fconfigure fix in this patch will still be
relevant even after such a fix to the implementation of this proc.

> diff --git a/lib/branch.tcl b/lib/branch.tcl 
> index 777eeb7..8b0c485 100644
> --- a/lib/branch.tcl
> +++ b/lib/branch.tcl
> @@ -8,6 +8,7 @@ proc load_all_heads {} {
>  	set rh_len [expr {[string length $rh] + 1}]
>  	set all_heads [list]
>  	set fd [git_read for-each-ref --format=%(refname) $rh]
> +	fconfigure $fd -translation binary -encoding utf-8
>  	while {[gets $fd line] > 0} {
>  		if {!$some_heads_tracking || ![is_tracking_branch $line]} {
>  			lappend all_heads [string range $line $rh_len end]
> @@ -24,6 +25,7 @@ proc load_all_tags {} {
>  		--sort=-taggerdate \
>  		--format=%(refname) \
>  		refs/tags]
> +	fconfigure $fd -translation binary -encoding utf-8
>  	while {[gets $fd line] > 0} {
>  		if {![regsub ^refs/tags/ $line {} name]} continue
>  		lappend all_tags $name

  reply	other threads:[~2019-12-09 19:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-07  0:29 [PATCH] git gui: fix branch name encoding error on git gui 加藤一博
2019-12-09 19:15 ` Junio C Hamano [this message]
2019-12-09 21:09   ` Pratyush Yadav
2019-12-09 21:25     ` Junio C Hamano
2019-12-11 16:05 ` Pratyush Yadav

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

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

  git send-email \
    --in-reply-to=xmqqpngxux78.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=kato-k@ksysllc.co.jp \
    --cc=me@yadavpratyush.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.
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 NNTP newsgroup(s).