search

git-for-windows / git

A fork of Git containing Windows-specific patches.
http://gitforwindows.org/
Other
8.21k stars 2.5k forks source link

gitk: For tags the name of the author with UTF-8 characters will not be displayed correctly. #767

Closed bitjo closed 4 years ago

bitjo commented 8 years ago 
$ uname
MINGW64_NT-6.1
$ uname
MINGW64_NT-6.1

defaults

no

Details

tty/bash

$ git clone https://github.com/bitjo/git-issue1
$ cd git-issue1/
$ gitk

After click on Tag1 in gitk I should see:

Tag: Tag1
object 4a34266866646ef1d722f3ca5fe89f987e26ceef
type commit
tag Tag1
tagger Groß <gross@example.com> 1464276169 +0200

Tag message

gitk shows:

Tag: Tag1
object 4a34266866646ef1d722f3ca5fe89f987e26ceef
type commit
tag Tag1
tagger Groß <gross@example.com> 1464276169 +0200

Tag message

The taggers name in line 5 (Groß) is not correctly (Groß).

see above: https://github.com/bitjo/git-issue1

dscho commented 7 years ago

Which version of Git for Windows are you using? 32-bit or 64-bit? Include the output of git version as well.

$ uname MINGW64_NT-6.1

That is not the git version.

In any case, I think this may be fixed in the meantime, as I could not reproduce it with v2.11.1.

bitjo commented 7 years ago

The issue isn't fiexed:

$ git clone https://github.com/bitjo/git-issue1
$ cd git-issue1/
$ gitk

gitk screenshot, note red text: screenshot gitk

Version infos:

user@ADMIX MINGW64 /t/tmp/git-issue1 (master)
$ uname -a
MINGW64_NT-6.1 ADMIX 2.6.1(0.306/5/3) 2017-01-20 15:23 x86_64 Msys
user@ADMIX MINGW64 /t/tmp/git-issue1 (master)
$ git version
git version 2.11.1.windows.1

An expected screenshot: screenshot gitk unix

dscho commented 7 years ago

This is how it looks here:

gitk-encoding-issue-767

bitjo commented 7 years ago

@dscho

You have to click on 'Tag1' to see the malformed output (don't click on branch or commit summary).

This bug is related to #761.

There is a basic problem with the endcoding in Git Gui / Gitk on Windows. It's probably easy to solve by a TcL / TK expert.

I'm not that expert, but I know that this bug, #761. and more unpublished bugs have the same cause.

bitjo commented 7 years ago

BTW @dscho

I do not know github well.

What do I need to do to make my images look inline?

PhilipOakley commented 7 years ago

I had a look at the repo. Cloned as instructed. Here are screen shots of gitk with Tag1 (yellow) clicked - note that the 'Tagger' (lower left) is poorly formed.

bad gitk tagger

Also note gitk has no font encoding preference (git gui does).

Now here is the same in the mintty window (I keep bash separate from cmd, and it's an XP netbook - still works!!)

git cat-file -p Tag1

bad gitk tagger mintty

(took me a bit to get the right tag foo invocation!)

See that the tagger name is correctly formed here. So problem [gitk fails to render tagger names correctly] repeated!

Just need some spare time to look at gitk's code.

For inserting the images see the line just below the writing pane "Attach files by dragging & dropping, selecting them, or pasting from the clipboard. " I did the Alt-PrtScn snapshot and dropped it into paint then saved as a png, before using the select.

bitjo commented 7 years ago

(For inserting the images ... I had linked to the Github pages containing the image. This is bad. I need to link to the URL of the image.)

PhilipOakley commented 7 years ago

https://github.com/git/git/blob/master/gitk-git/gitk#L11262-L11277 

proc add_tag_ctext {tag} {
    global ctext cached_tagcontent tagids

    if {![info exists cached_tagcontent($tag)]} {
    catch {
        set cached_tagcontent($tag) [exec git cat-file -p $tag]
    }
    }
    $ctext insert end "[mc "Tag"]: $tag\n" bold
    if {[info exists cached_tagcontent($tag)]} {
    set text $cached_tagcontent($tag)
    } else {
    set text "[mc "Id"]:  $tagids($tag)"
    }
    appendwithlinks $text {}
}

The two lines set cached_tagcontent($tag) [exec git cat-file -p $tag] and set text $cached_tagcontent($tag) look to be the part that accesses the tag and gets its text.

Not sure yet about how the equivalent commit text info is aquired for dispay in the same window, which may show how to do the displaying right (or at least how to tweak stuff).

PhilipOakley commented 7 years ago

The regular commit display starts at https://github.com/git/git/blob/master/gitk-git/gitk#L7261 proc selectline {l isnew {desired_loc {}} {switch_to_patch 0}} { (found by searching for the "Follow" line (L7392) that is added in the display.

We then see the author at https://github.com/git/git/blob/master/gitk-git/gitk#L7348 $ctext insert end "[mc "Author"]: [lindex $info 1] $date\n"

So now search for how $info was generated.

PhilipOakley commented 7 years ago

try https://github.com/git/git/blob/master/gitk-git/gitk#L7343-L7346 to see its loaded via getcommit($id)

https://github.com/git/git/blob/master/gitk-git/gitk#L1736 proc getcommit {id} {

bed time. time for someone else to pick up the thread...

PhilipOakley commented 7 years ago

A bit more Yak-shaving on this.

Intersting point of note: the info exists commitdata($id) and the [lindex $info 1] are completely different uses of info.

There is a Tcl command called info, with subcommand exists that will tell you if a variable exists, so you can see if you need to create or set it. http://wiki.tcl.tk/1185

Meanwhile parts of the gitk code also create a variable called info which has higher priority in the local scope and would then hide the info command (because tcl is interpreted...). Thankfully the info variable is tightly scoped but it's something to look out for.

Back to the https://github.com/git/git/blob/master/gitk-git/gitk#L1736 proc getcommit {id}

This either parsecommit a commit it knows, or readcommit (with post parsing) for ones it doesn't know

https://github.com/git/git/blob/master/gitk-git/gitk#L1671-L1698 proc parsecommit (there is more after that range)

At lines https://github.com/git/git/blob/master/gitk-git/gitk#L1686-L1687 of parsecommit we see that the header and comment parts of the commit object are set.

They both use the string command keyword. Maybe this is it. Didn't notice a string keyword for the tag message (https://github.com/git/git/blob/master/gitk-git/gitk#L11272).

Need to double check the string command, and the appendwithlinks $text {} https://github.com/git/git/blob/master/gitk-git/gitk#L11272 in the tag processing.

PhilipOakley commented 7 years ago

In http://wiki.tcl.tk/1500 section Using string functions for binary data it says:

The following subcommands force promotion to unicode strings:

string first
string last
string map
string replace
string reverse

-- Try http://wiki.tcl.tk/_//search?submit=Search&S=ByteArray+UTF&_charset_=UTF-8 to review what they do internally to hide the encoding issues of Everything is a string

PhilipOakley commented 7 years ago

Did a quick try at wrapping the line that gets the tag data set cached_tagcontent($tag) [exec git cat-file -p $tag] with a [string range <str> 0 end] but no luck.

So, Back to the other side of the commit data. the `do_readcommit procedure. And, lo and behold, it has loads of stuff on encoding.

Looked at the code that reads the commit data git log --no-color --pretty=raw -1 and tried to find a revision notation git help revisions here for the tag itself, but no luck there.

So it has to be the special git cat-file -p Tag1 format` hence we'll need to repeat the do_readcommit logic, or at least pass in the base command string !

PhilipOakley commented 7 years ago

two commits at https://github.com/PhilipOakley/gitk/tree/tagger on top of the master I had.

Took a bit to get the second inovcation right. needed to drop the { } brackets in the set contents [do_readgitcmd $tag $basecmd].

dscho commented 6 years ago

@bitjo you seemed interested in this issue. I will reopen, expecting you to brush up @PhilipOakley's patches and open a clean and polished Pull Request.

dscho commented 6 years ago

I will reopen, expecting you to brush up @PhilipOakley's patches and open a clean and polished Pull Request.

@bitjo ping.

dscho commented 6 years ago

@bitjo happy new year. And ping. You wanted this, right?

dscho commented 5 years ago

I guess not.

bitjo commented 5 years ago

@dscho (Sorry to English readers: I just write German, because @scho understands it and I wanted to save time. DL; DR: I do not have time to submit the patch.)

Ich schaffe es zeitlich nicht, weil dieses Ticket für mich keine hohe Priorität hat.

Der Patch von @PhilipOakley funktionierte bei mir mir lokal.

Um den Patch als Pull Request einzureichen, fehlt mir aber die Zeit. Ich müsste mich mit der der Art und Weise der Einreichung und den geforderten Richtlinien zur Qualität im Projekt auseinandersetzen. Dazu fehlt mir aber derzeit die Zeit.

Ich bitte um Verzeihung, dass ich hier nicht aktiv werde. Aber ich sehe es schon als guten Beitrag an, dass ich das Ticket überhaupt geöffnet habe.

Aus meiner Sicht ist #761 viel bedeutender, da dort nicht nur in git gui/gitk etwas falsch dargestellt wird, sondern das Repo falsch geändert wird.

dscho commented 5 years ago

@PhilipOakley do you have the time to brush up those commits? If not, I suggest to just close this ticket (if it was important enough, somebody would have found the time to address this).

PhilipOakley commented 5 years ago

Hi @dscho At the moment I'm a bit thin on time. I may get more time from 2019, assuming retirement goes well...

The priority on this one is probably elsewhere as there is still no maintainer/curator for the git-gui/gitk repos anyway.

Were there any changes that needed brushing up on anyway? Or was it just a case of re-validation?