{"thread":{"id":"52397","subject":"[PATCH] git gui: fix branch name encoding error on git gui","startedAt":"2019-12-07T00:29:16Z","lastAt":"2019-12-11T16:05:56Z","messageCount":5,"participants":["加藤一博","Junio C Hamano","Pratyush Yadav"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"387651","messageId":"20191207002842.32208-1-kato-k@ksysllc.co.jp","threadId":"52397","inReplyTo":null,"subject":"[PATCH] git gui: fix branch name encoding error on git gui","fromName":"加藤一博","fromEmail":"kato-k@ksysllc.co.jp","sentAt":"2019-12-07T00:29:09Z","receivedAt":"2019-12-07T00:29:16Z","isPatch":true,"sender":{"key":"kato-k@ksysllc.co.jp","avatar":"https://avatars.githubusercontent.com/u/5441449?v=4"},"body":"After \"git checkout -b '漢字'\" to create a branch with UTF-8\ncharacter in it, \"git gui\" shows the branch name incorrectly,\nas it forgets to turn the bytes\nread from the \"git for-each-ref\" and\nread from \"HEAD\" file\ninto Unicode characters.\n\nSigned-off-by: Kazuhiro Kato <kato-k@ksysllc.co.jp>\n---\n git-gui.sh     | 1 +\n lib/branch.tcl | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 0d21f56..8f4a9ae 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -684,6 +684,7 @@ proc load_current_branch {} {\n \tglobal current_branch is_detached\n \n \tset fd [open [gitdir HEAD] r]\n+\tfconfigure $fd -translation binary -encoding utf-8\n \tif {[gets $fd ref] < 1} {\n \t\tset ref {}\n \t}\ndiff --git a/lib/branch.tcl b/lib/branch.tcl\nindex 777eeb7..8b0c485 100644\n--- a/lib/branch.tcl\n+++ b/lib/branch.tcl\n@@ -8,6 +8,7 @@ proc load_all_heads {} {\n \tset rh_len [expr {[string length $rh] + 1}]\n \tset all_heads [list]\n \tset fd [git_read for-each-ref --format=%(refname) $rh]\n+\tfconfigure $fd -translation binary -encoding utf-8\n \twhile {[gets $fd line] > 0} {\n \t\tif {!$some_heads_tracking || ![is_tracking_branch $line]} {\n \t\t\tlappend all_heads [string range $line $rh_len end]\n@@ -24,6 +25,7 @@ proc load_all_tags {} {\n \t\t--sort=-taggerdate \\\n \t\t--format=%(refname) \\\n \t\trefs/tags]\n+\tfconfigure $fd -translation binary -encoding utf-8\n \twhile {[gets $fd line] > 0} {\n \t\tif {![regsub ^refs/tags/ $line {} name]} continue\n \t\tlappend all_tags $name\n-- \n2.20.1.windows.1\n\n"},{"id":"387819","messageId":"xmqqpngxux78.fsf@gitster-ct.c.googlers.com","threadId":"52397","inReplyTo":"20191207002842.32208-1-kato-k@ksysllc.co.jp","subject":"Re: [PATCH] git gui: fix branch name encoding error on git gui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-09T19:15:55Z","receivedAt":"2019-12-09T19:16:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"加藤一博 <kato-k@ksysllc.co.jp> writes:\n\n> After \"git checkout -b '漢字'\" to create a branch with UTF-8\n> character in it, \"git gui\" shows the branch name incorrectly,\n> as it forgets to turn the bytes\n> read from the \"git for-each-ref\" and\n> read from \"HEAD\" file\n> into Unicode characters.\n\nThanks.\n\nNote to the git-gui mentainer.  The above may want to be\nline-wrapped a bit.\n\n> Signed-off-by: Kazuhiro Kato <kato-k@ksysllc.co.jp>\n> ---\n>  git-gui.sh     | 1 +\n>  lib/branch.tcl | 2 ++\n>  2 files changed, 3 insertions(+)\n>\n> diff --git a/git-gui.sh b/git-gui.sh\n> index 0d21f56..8f4a9ae 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> @@ -684,6 +684,7 @@ proc load_current_branch {} {\n>  \tglobal current_branch is_detached\n>  \n>  \tset fd [open [gitdir HEAD] r]\n> +\tfconfigure $fd -translation binary -encoding utf-8\n>  \tif {[gets $fd ref] < 1} {\n>  \t\tset ref {}\n>  \t}\n\nA comment totally outside the scope of this fix to anybody\ninterested in further working on this code.\n\nThis piece of code is way too intimate with the implementation\ndetails of HEAD and yet not intimate enough to know that HEAD can be\na symlink (in other words, it is a poor imitation of the real logic\nimplemented in git core).  A kosher way to implement this would be\nto call\n\n\tgit symbolic-ref --quiet --short HEAD\n\nwhich would succeed and give the branch name to its standard output,\nor would fail when the head is detached.  Set \"current_branch\" and\n\"is_detached\" according to the outcome.\n\nAnd yes, Kato-san's fconfigure fix in this patch will still be\nrelevant even after such a fix to the implementation of this proc.\n\n> diff --git a/lib/branch.tcl b/lib/branch.tcl \n> index 777eeb7..8b0c485 100644\n> --- a/lib/branch.tcl\n> +++ b/lib/branch.tcl\n> @@ -8,6 +8,7 @@ proc load_all_heads {} {\n>  \tset rh_len [expr {[string length $rh] + 1}]\n>  \tset all_heads [list]\n>  \tset fd [git_read for-each-ref --format=%(refname) $rh]\n> +\tfconfigure $fd -translation binary -encoding utf-8\n>  \twhile {[gets $fd line] > 0} {\n>  \t\tif {!$some_heads_tracking || ![is_tracking_branch $line]} {\n>  \t\t\tlappend all_heads [string range $line $rh_len end]\n> @@ -24,6 +25,7 @@ proc load_all_tags {} {\n>  \t\t--sort=-taggerdate \\\n>  \t\t--format=%(refname) \\\n>  \t\trefs/tags]\n> +\tfconfigure $fd -translation binary -encoding utf-8\n>  \twhile {[gets $fd line] > 0} {\n>  \t\tif {![regsub ^refs/tags/ $line {} name]} continue\n>  \t\tlappend all_tags $name\n"},{"id":"387844","messageId":"20191209210942.gcyp6ch7zingkhfs@yadavpratyush.com","threadId":"52397","inReplyTo":"xmqqpngxux78.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] git gui: fix branch name encoding error on git gui","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-12-09T21:09:42Z","receivedAt":"2019-12-09T21:09:51Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 09/12/19 11:15AM, Junio C Hamano wrote:\n> 加藤一博 <kato-k@ksysllc.co.jp> writes:\n> \n> > After \"git checkout -b '漢字'\" to create a branch with UTF-8\n> > character in it, \"git gui\" shows the branch name incorrectly,\n> > as it forgets to turn the bytes\n> > read from the \"git for-each-ref\" and\n> > read from \"HEAD\" file\n> > into Unicode characters.\n> \n> Thanks.\n> \n> Note to the git-gui mentainer.  The above may want to be\n> line-wrapped a bit.\n\nThanks. Already done :)\n \n> > Signed-off-by: Kazuhiro Kato <kato-k@ksysllc.co.jp>\n> > ---\n> >  git-gui.sh     | 1 +\n> >  lib/branch.tcl | 2 ++\n> >  2 files changed, 3 insertions(+)\n> >\n> > diff --git a/git-gui.sh b/git-gui.sh\n> > index 0d21f56..8f4a9ae 100755\n> > --- a/git-gui.sh\n> > +++ b/git-gui.sh\n> > @@ -684,6 +684,7 @@ proc load_current_branch {} {\n> >  \tglobal current_branch is_detached\n> >  \n> >  \tset fd [open [gitdir HEAD] r]\n> > +\tfconfigure $fd -translation binary -encoding utf-8\n> >  \tif {[gets $fd ref] < 1} {\n> >  \t\tset ref {}\n> >  \t}\n> \n> A comment totally outside the scope of this fix to anybody\n> interested in further working on this code.\n> \n> This piece of code is way too intimate with the implementation\n> details of HEAD and yet not intimate enough to know that HEAD can be\n> a symlink (in other words, it is a poor imitation of the real logic\n> implemented in git core).  A kosher way to implement this would be\n> to call\n> \n> \tgit symbolic-ref --quiet --short HEAD\n> \n> which would succeed and give the branch name to its standard output,\n> or would fail when the head is detached.  Set \"current_branch\" and\n> \"is_detached\" according to the outcome.\n\nIt was introduced in fc4e8da (git-gui: Internalize symbolic-ref HEAD \nreading logic, 2007-05-30). The commit message is:\n\n  To improve performance on fork+exec impoverished systems (such as\n  Windows) we want to avoid running git-symbolic-ref on every rescan\n  if we can do so.  A quick way to implement such an avoidance is to\n  just read the HEAD ref ourselves; we'll either see it as a symref\n  (starts with \"ref: \") or we'll see it as a detached head (40 hex\n  digits).  In either case we can treat that as our current branch.\n\nNow I'm not sure how relevant this still is over 12 years later, but \nAFAIK a fork+exec is still very costly on Windows.\n\nSo I wonder whether we should manually check if HEAD is a symbolic link \nor we should just use git-symbolic-ref and hope the performance doesn't \ndrop too much.\n \n> And yes, Kato-san's fconfigure fix in this patch will still be\n> relevant even after such a fix to the implementation of this proc.\n\nMe and Jonathan (Cc) have been having a discussion [0] about whether \nhard-coding UTF-8 as the refname encoding is the right idea. The gist of \nit is that Git _technically_ allows refnames to be in other encodings as \nlong as the strings are NULL terminated. It does not restrict itself to \nvalid UTF-8 only. More details in the linked thread, of course.\n\nMy position is that we should default to UTF-8 given its popularity (at \nleast in the Git world), but I'm wondering whether we should also add a \nconfig variable to allow users to configure their encodings.\n\nIf you don't mind, your thoughts on this would be appreciated :)\n\n[0] https://github.com/prati0100/git-gui/pull/21\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"387846","messageId":"xmqqr21dtcn7.fsf@gitster-ct.c.googlers.com","threadId":"52397","inReplyTo":"20191209210942.gcyp6ch7zingkhfs@yadavpratyush.com","subject":"Re: [PATCH] git gui: fix branch name encoding error on git gui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-09T21:25:16Z","receivedAt":"2019-12-09T21:25:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pratyush Yadav <me@yadavpratyush.com> writes:\n\n> My position is that we should default to UTF-8 given its popularity (at \n> least in the Git world), but I'm wondering whether we should also add a \n> config variable to allow users to configure their encodings.\n>\n> If you don't mind, your thoughts on this would be appreciated :)\n>\n> [0] https://github.com/prati0100/git-gui/pull/21\n\nQuite honestly, as long as we can keep the core part to stay\n\"pathnames and refnames are just runs of bytes, terminated by NUL,\nand no other interpretation is necessary\", I do not think I care too\ndeeply if the UI layers (either GUI or Porcelain) become a bit more\nrestrictive.\n\nBut that is just my gut feeling.\n\n\n"},{"id":"387947","messageId":"20191211160546.m2qlxg23on7itvgw@yadavpratyush.com","threadId":"52397","inReplyTo":"20191207002842.32208-1-kato-k@ksysllc.co.jp","subject":"Re: [PATCH] git gui: fix branch name encoding error on git gui","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-12-11T16:05:46Z","receivedAt":"2019-12-11T16:05:56Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 07/12/19 12:29AM, 加藤一博 wrote:\n> After \"git checkout -b '漢字'\" to create a branch with UTF-8\n> character in it, \"git gui\" shows the branch name incorrectly,\n> as it forgets to turn the bytes\n> read from the \"git for-each-ref\" and\n> read from \"HEAD\" file\n> into Unicode characters.\n> \n> Signed-off-by: Kazuhiro Kato <kato-k@ksysllc.co.jp>\n\nMerged. Thanks.\n\n-- \nRegards,\nPratyush Yadav\n"}]}