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).

> 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...

>                       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

Reply via email to