Thread (11 messages) flat view 11 messages, 4 authors, 2017-12-14

Re: [PATCH v2] git-gui: Prevent double UTF-8 conversion

From: Johannes Schindelin <hidden>
Date: 2017-12-04 14:31:49

Hi Łukasz,

On Sat, 2 Dec 2017, Łukasz Stelmach wrote:
Convert author's name from the UTF-8 (or any other) encoding in
load_last_commit function the same way commit message is converted.

Amending commits in git-gui without such conversion breaks UTF-8
strings. For example, "\305\201ukasz" (as written by git cat-file) becomes
"\303\205\302\201ukasz" in an amended commit.
Okay, that makes the issue a lot clearer to me (the explicit mention of
"author's name", that is).
quoted hunk ↗ jump to hunk
diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
index 83620b7cb..f820c24bf 100644
--- a/git-gui/lib/commit.tcl
+++ b/git-gui/lib/commit.tcl
@@ -34,9 +34,7 @@ You are currently in the middle of a merge that has not been fully completed.  Y
 					lappend parents [string range $line 7 end]
 				} elseif {[string match {encoding *} $line]} {
 					set enc [string tolower [string range $line 9 end]]
-				} elseif {[regexp "author (.*)\\s<(.*)>\\s(\\d.*$)" $line all name email time]} {
-					set commit_author [list name $name email $email date $time]
-				}
+				} elseif {[regexp "author (.*)\\s<(.*)>\\s(\\d.*$)" $line all name email time]} { }
 			}
This looks wrong, as the commit_author would now also be set if the header
was not found (mind you, this would make for an incorrect Git commit, but
the code explicitly tries to set commit_author only in the case that the
author line was found.

But we cannot set commit_author here because the encoding is read as
another header line (and in a valid commit object, the encoding line (if
any) has to be *below* the author line).

So it *has* to be this way. Maybe mention this in the commit message, to
avoid head-scratching?

However, I would still recommend to `set name ""` before the loop parsing
the header, and...
quoted hunk ↗ jump to hunk
 			set msg [read $fd]
 			close $fd
@@ -44,7 +42,9 @@ You are currently in the middle of a merge that has not been fully completed.  Y
 			set enc [tcl_encoding $enc]
 			if {$enc ne {}} {
 				set msg [encoding convertfrom $enc $msg]
+				set name [encoding convertfrom $enc $name]
 			}
+			set commit_author [list name $name email $email date $time]
Guarding this assignment in an `if {$name ne ""} { ... }`, just in case.
 			set msg [string trim $msg]
 		} err]} {
 		error_popup [strcat [mc "Error loading commit data for amend:"] "\n\n$err"]
Thanks,
Johannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help