Skip to contentSkip to conversation

gitk: Fixing file name encoding issues.

PATCH In review Kazuhiro Kato via GitGitGadget 3 participants 7 messages Feb 18, 2025 – Feb 19, 2025

lore
Share & tools
lore

Kazuhiro Kato via GitGitGadget

author
#0 lore raw

cover gitk: Fixing file name encoding issues.

fix: file name encoding issues. fix: when resolving merge conflicts, japanese file names become garbled.

Kazuhiro Kato (2):
Fixing file name encoding issues.
fix: when resolving merge conflicts, japanese file names become
garbled.

 gitk-git/gitk | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1886%2Fkkato233%2Ffix_filename_encoding-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1886/kkato233/fix_filename_encoding-v1
Pull-Request: https://github.com/git/git/pull/1886

Signature · 1 line
gitgitgadget
author
#1 lore raw

PATCH 1/2 Fixing file name encoding issues.

From: Kazuhiro Kato <[email protected]>

Signed-off-by Kazuhiro Kato <[email protected]>
---
 gitk-git/gitk | 1 +
 1 file changed, 1 insertion(+)
gitk-git/gitk +1 -0
index 47a7c1d29c4..88951ed2384 100755
@@ -12379,6 +12379,7 @@ catch {
 if {$gitencoding == ""} {
     set gitencoding "utf-8"
 }
+encoding system utf-8
 set tclencoding [tcl_encoding $gitencoding]
 if {$tclencoding == {}} {
     puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"
Signature · 1 line
gitgitgadget
author
#4 lore raw

PATCH 2/2 fix: when resolving merge conflicts, japanese file names become garbled.

In reply to Kazuhiro Kato via GitGitGadget

From: Kazuhiro Kato <[email protected]>

Signed-off-by Kazuhiro Kato <[email protected]>
---
 gitk-git/gitk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
gitk-git/gitk +4 -2
index 88951ed2384..f4f8dbd5fad 100755
@@ -8205,12 +8205,13 @@ proc parseblobdiffline {ids line} {
 
         if {$type eq "--cc"} {
             # start of a new file in a merge diff
-            set fname [string range $line 10 end]
+            set fname_raw [string range $line 10 end]
+            set fname [encoding convertfrom $fname_raw]
             if {[lsearch -exact $treediffs($ids) $fname] < 0} {
                 lappend treediffs($ids) $fname
                 add_flist [list $fname]
             }
-
+            set fname $fname_raw
         } else {
             set line [string range $line 11 end]
             # If the name hasn't changed the length will be odd,

@@ -8310,6 +8311,7 @@ proc parseblobdiffline {ids line} {
             set diffinhdr 0
             return
         }
+        set line [encoding convertfrom $line]
         $ctext insert end "$line\n" filesep
 
     } else {
Signature · 1 line
gitgitgadget
#6 lore raw
In reply to Kazuhiro Kato via GitGitGadget
Quoted 10 lines · Kazuhiro Kato via GitGitGadget

fix: file name encoding issues. fix: when resolving merge conflicts, japanese file names become garbled.

Kazuhiro Kato (2):
Fixing file name encoding issues.
fix: when resolving merge conflicts, japanese file names become
garbled.

 gitk-git/gitk | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Please

- base your changes to j6t's gitk repository

https://github.com/j6t/gitk

where the file your patch would touch should appear at the top level of the working tree. This "independent" history is then merged into my tree with "git pull -Xsubtree=gitk-git".

- have the e-mail address of the gitk maintainer on the Cc: line, i.e. "Cc: Johannes Sixt <[email protected]>".

Thanks.

cf.
https://lore.kernel.org/git/[email protected]/
https://lore.kernel.org/git/[email protected]/
https://lore.kernel.org/git/[email protected]/

Quoted 6 lines

base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1886%2Fkkato233%2Ffix_filename_encoding-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1886/kkato233/fix_filename_encoding-v1
Pull-Request: https://github.com/git/git/pull/1886

#2 lore raw

Re: [PATCH 1/2] Fixing file name encoding issues.

In reply to Kazuhiro Kato via GitGitGadget

On Tue, Feb 18, 2025 at 10:42:25PM +0000, Kazuhiro Kato via GitGitGadget wrote:

[...]

gitk-git/gitk catch { · line 12385

Quoted 12 lines
gitk-git/gitk +1 -0
index 47a7c1d29c4..88951ed2384 100755
@@ -12379,6 +12379,7 @@ catch {
 if {$gitencoding == ""} {
     set gitencoding "utf-8"
 }
+encoding system utf-8
 set tclencoding [tcl_encoding $gitencoding]
 if {$tclencoding == {}} {
     puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"

I'm not sure one should sensibly do this - except to implement some well-understood and well-documented kludge, simply because the "system encoding" is supposed to be set by the Tcl runtime.

Let's demonstrate (on a Linux-based system with UTF-8 locale):

tmp$ touch 'привет мир.txt'
tmp$ tclsh
% encoding system
utf-8
% glob привет*.txt
{привет мир.txt}
% encoding system cp1251
% encoding system
cp1251
% glob привет*.txt
no files matched glob pattern "привет*.txt"
%

Here, CP1251 is a Windows "code page" for Cyrillic; it's what [encoding system] reports on Windows systems.

Note that I create a file whose name is two words in Cyrillic script encoded in UTF-8, and Tcl is fine finding and dispaying this file (via its "glob" command). But as soon as I change the Tcl's system encoding to another 8-bit Cyrillic encoding, globbing stops working.

Moreover, if I'd do [glob *.txt] so that my file would be matched anyway, its named would not be readable since Tcl would re-encode it from CP1251 to Unicode, but the name is encoded in UTF-8, not CP1251.

In other words, your patch (supposedly) works on UTF-8-based systems which is common to Linux-based OSes and MacOS, but I'm afraid it won't work on Windows.

#3 lore raw

Re: [PATCH 1/2] Fixing file name encoding issues.

Quoted 16 lines · Konstantin Khomoutov

On Tue, Feb 18, 2025 at 10:42:25PM +0000, Kazuhiro Kato via GitGitGadget wrote:

[...]

gitk-git/gitk catch { · line 12385

Quoted 12 lines
gitk-git/gitk +1 -0
index 47a7c1d29c4..88951ed2384 100755
@@ -12379,6 +12379,7 @@ catch {
 if {$gitencoding == ""} {
     set gitencoding "utf-8"
 }
+encoding system utf-8
 set tclencoding [tcl_encoding $gitencoding]
 if {$tclencoding == {}} {
     puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"
Quoted 7 lines

I'm not sure one should sensibly do this - except to implement some well-understood and well-documented kludge, simply because the "system encoding" is supposed to be set by the Tcl runtime.
...
In other words, your patch (supposedly) works on UTF-8-based systems which is common to Linux-based OSes and MacOS, but I'm afraid it won't work on Windows.

In other words, things should work without "encoding system blah" forcing a particular encoding that Tcl may not agree with? Would this mean perhaps in a "curious" repository with paths encoded in something Tcl does not expect to be used (e.g., on a UTF-8 system somehow EUC-jp is used for paths containing Japanese characters), it needs to be possible to specify a "curious" encoding either with an end-user on-demand action (e.g., menu items) or with an repository configuration (e.g., gitk.pathencoding = euc-jp)?

Thanks.

#5 lore raw

Re: [PATCH 2/2] fix: when resolving merge conflicts, japanese file names become garbled.

In reply to Kazuhiro Kato via GitGitGadget
Quoted 1 line · Kazuhiro Kato via GitGitGadget

From: Kazuhiro Kato <[email protected]>

Here is a place to give a bit more context. In what way the current code is wrong, what end-user visible symptoms are brought due to that wrongness, what is the correct way to implement it, etc.

Quoted 10 lines
Signed-off-by Kazuhiro Kato <[email protected]>
---
 gitk-git/gitk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
gitk-git/gitk +0 -0
index 88951ed2384..f4f8dbd5fad 100755

gitk-git/gitk proc parseblobdiffline {ids line} { · line 8209

Quoted 6 lines
gitk-git/gitk +2 -1
@@ -8205,12 +8205,13 @@ proc parseblobdiffline {ids line} {
 
         if {$type eq "--cc"} {
             # start of a new file in a merge diff
-            set fname [string range $line 10 end]
+            set fname_raw [string range $line 10 end]
+            set fname [encoding convertfrom $fname_raw]

Is this "the Tcl read from git things as sequence of bytes, not characters, so somebody needs to pass the bytes to "encoding" function to turn them into a sequence of characters? Unless everything is US-ASCII, that is.

If that is the case, presumably the $line has a sequence of bytes, so it may be wrong to chop it at 10th position (presumably that's 10th byte, not 10th character) when we are trying to teach the code to deal with non-ASCII data, no?

I am reasonably sure that [string length "diff --git"] is where 10 comes from, and that prefix will always be in ASCII, but it feels safer and kosher if we converted the whole line first and then chopped off the prefix.

The patch title says Japanese, but I would imagine this applies to anything non-ASCII, so it would be better to retitle the patch to say "non-ASCII" instead to signal that the issue the patch fixes applies more widely.

Thanks.