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

Correct rune width #21

Closed
wants to merge 1 commit into from
Closed

Correct rune width #21

wants to merge 1 commit into from

Conversation

mattn
Copy link
Contributor

@mattn mattn commented May 1, 2013

Thanks for your working.
Currently, godit doesn't treat double width cell characters correctly.
This is related in termbox-go. This change correct position of draw.
I'll send another pull-req to godit.

@mattn mattn mentioned this pull request May 1, 2013
@nsf
Copy link
Owner

nsf commented May 1, 2013

Well. It looks too simple to be truth. I'll check that out and the godit part soon (but don't expect it too soon, sometime on this week).

By putting runeWidth into the termbox_windows.go I assume you haven't tested it on linux/osx/etc as it shouldn't even compile.

@nsf
Copy link
Owner

nsf commented May 4, 2013

I've added CJK support for terminal-based termbox-go implementations and your patch to godit. It seems to work properly so far.

As far as I understand you're using windows OS mainly, but if you can test it on linux/macosx it would be nice.

Windows implementation will follow shortly.

@nsf
Copy link
Owner

nsf commented May 4, 2013

Windows implementation isn't as simple as it seems. Will take another day or two.

@nsf
Copy link
Owner

nsf commented May 6, 2013

Windows implementation is here, please test it. There could be some input issues though. Like pasting from the clipboard (ConEmu supports that, and it doesn't work properly).

@mattn
Copy link
Contributor Author

mattn commented May 7, 2013

Sorry for delay. I was in holidays. I'll look into it. Thank you.

@mattn
Copy link
Contributor Author

mattn commented May 7, 2013

I updated termbox-go to latest master branch. And build godit with my rune_width branch.
Below is a file editing with vim.

And godit

Cursor is on . Then If type right-cursor on there, cursor moved next line.

I guess, godit treat cursor correctly, but displaying is wrong.

@nsf
Copy link
Owner

nsf commented May 7, 2013

I see, well since my windows has no support for CJK runes, I use ConEmu to test godit/termbox. And in there I draw CJK runes using the rune itself and a space character, because otherwise they are displayed in a very ugly fashion (overlapping each other). But it seems that this is exactly a problem in your case. I'll try to figure that out.

There is also another problem with some characters which are greater than 0x10000 in unicode (they are encoded as two utf16 codepoints). And in their case there is no need to insert a space.

Anyways, probably ConEmu is quite broken, I'll take a look at emacs/vim implementations as well to see how they do it.

@nsf
Copy link
Owner

nsf commented May 7, 2013

Btw, I've added the rune_width handling stuff in the godit as well, no need to use your branch anymore. But I haven't closed the issue because for now I can't say if it works or not. :)

Also take a look at _demos/output.go in termbox-go, I've added some CJK runes here. A screenshot would be nice. There's just drawing.

@nsf
Copy link
Owner

nsf commented May 11, 2013

Well, I tried installing japanese locale onto my windows and see how things work there. And it's quite horrible, in a sense that not just CJK runes have different widths but seems like some of the unicode box-drawing symbols as well.

Since I have no interest in that at all, I'll be working very slowly on this. If you want to speed things up, provide patches (working patches).

Currently I'm starting to lose hope that it's even possible to make it portable. Because for example if it's true that box-drawing characters are wider in fonts in Japanese locale, then it creates even more problems for people who write end user applications. It's ok to be aware that ordinary letters may take more than one cell, but for box-drawing characters it means you'll have to reevaluate the whole UI layout thing. The other issue is that there are different console implementation on windows it seems. Like for example this ConEmu app, I can certainly see that it deals with CJK runes in its own fashion (I use GNU unifont in there).

Oh and crossplatform portability makes it worse. What if box-drawing characters on linux take one cell and on windows they take two. Sounds like a nightmare.

@mattn
Copy link
Contributor Author

mattn commented Aug 23, 2013

It seems this doesn't fixes yet. And it seems prepare_diff_messages seems to have problem. If I change like follow, _demo/output.go, godit is working good.

diff --git a/termbox_windows.go b/termbox_windows.go
index 8cc2b0a..f4163a6 100644
--- a/termbox_windows.go
+++ b/termbox_windows.go
@@ -421,6 +421,7 @@ func prepare_diff_messages() {
                    Fg: back.Fg,
                    Bg: back.Bg,
                }
+               continue
            }
            x += w
        }

@nsf
Copy link
Owner

nsf commented Aug 23, 2013

I'll take a look at that a bit later.

@mattn
Copy link
Contributor Author

mattn commented Nov 11, 2013

@nsf Can you handle this issue?

@nsf
Copy link
Owner

nsf commented Nov 11, 2013

I can apply the patch above (which adds "continue" statement), but I will not be able to test it anyway, so I don't see much sense in that. But if want to I can do that.

@mattn
Copy link
Contributor Author

mattn commented Jun 25, 2014

Latest termbox fixes this issue. I close this. I don't know what commit is done.
Thank you

@mattn mattn closed this Jun 25, 2014
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