Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cjk width #74

Merged
merged 2 commits into from
Mar 17, 2015
Merged

Fix cjk width #74

merged 2 commits into from
Mar 17, 2015

Conversation

mattn
Copy link
Contributor

@mattn mattn commented Mar 17, 2015

Related issue #69 #72

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

termui

works fine

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

peco

works fine too

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

@nsf do you know the another famous product which should be check this change?

@mattn mattn deleted the fix-cjk-width branch March 17, 2015 00:26
@nsf
Copy link
Owner

nsf commented Mar 17, 2015

Our own _demos/output.go has some cjk characters in it. But no, have no idea.

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

Is this your expected?

@nsf
Copy link
Owner

nsf commented Mar 17, 2015

Yes, looks good. But that's how it looks on my machine (that's what I meant by "it doesn't work for me"):

termboxcjk

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

What is your code page?

@nsf
Copy link
Owner

nsf commented Mar 17, 2015

What do you mean code page? It's a typical windows setup with russian/english locales.

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

please type chcp on cmd.exe

@nsf
Copy link
Owner

nsf commented Mar 17, 2015

Says 866.

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

@nsf because cell width is hard-coded as 2. https://github.com/nsf/termbox-go/blob/master/_demos/output.go#L57

@nsf
Copy link
Owner

nsf commented Mar 17, 2015

@mattn But what does it change? I mean when font doesn't support CJK runes things get broken?

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

ok, I could reproduce your problem.

chcp 866

I'll look into it.

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

It seems that prepare_diff_messages should be fixed I think.

@mattn
Copy link
Contributor Author

mattn commented Mar 17, 2015

About making diffs, diffs should be separated per lines. because columns is not width in CJK.

diff --git a/termbox_windows.go b/termbox_windows.go
index 62ac3fa..45b69e5 100644
--- a/termbox_windows.go
+++ b/termbox_windows.go
@@ -456,6 +456,14 @@ const (
    surr_self        = 0x10000
 )

+func rune_width(r rune) int {
+   w := runewidth.RuneWidth(r)
+   if w == 0 || w == 2 && runewidth.IsAmbiguousWidth(r) {
+       w = 1
+   }
+   return w
+}
+
 func append_diff_line(y int) int {
    n := 0
    for x := 0; x < front_buffer.width; {
@@ -463,16 +471,10 @@ func append_diff_line(y int) int {
        back := &back_buffer.cells[cell_offset]
        front := &front_buffer.cells[cell_offset]
        attr, char := cell_to_char_info(*back)
-       w := runewidth.RuneWidth(back.Ch)
-       if w == 0 {
-           w = 1
-       }
-       if w == 0 || w == 2 && runewidth.IsAmbiguousWidth(back.Ch) {
-           w = 1
-       }
        charbuf = append(charbuf, char_info{attr: attr, char: char[0]})
-       n++
        *front = *back
+       n++
+       w := rune_width(back.Ch)
        x += w
    }
    return n
@@ -498,25 +500,17 @@ func prepare_diff_messages() {
                break
            }
        }
-       if same && diff.lines > 0 {
-           diffbuf = append(diffbuf, diff)
-           diff = diff_msg{}
-       }
        if !same {
+           diff = diff_msg{}
            beg := len(charbuf)
            end := beg + append_diff_line(y)
-           if diff.lines == 0 {
-               diff.pos = short(y)
-               gbeg = beg
-           }
-           diff.lines++
+           diff.lines = 1
+           diff.pos = short(y)
+           gbeg = beg
            diff.chars = charbuf[gbeg:end]
+           diffbuf = append(diffbuf, diff)
        }
    }
-   if diff.lines > 0 {
-       diffbuf = append(diffbuf, diff)
-       diff = diff_msg{}
-   }
 }

 func cell_to_char_info(c Cell) (attr word, wc [2]wchar) {

But it seems to be problem in another part.

@pomutemu
Copy link

Microsoft Windows [Version 10.0.10074]
(c) 2015 Microsoft Corporation. All rights reserved.

C:\Windows\system32>chcp 932
現在のコード ページ: 932

Gyazo

@mattn
Copy link
Contributor Author

mattn commented Jun 16, 2015

Probably your font is not rasterfont. right?

@pomutemu
Copy link

You are right.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants