search

jroimartin / gocui

Minimalist Go package aimed at creating Console User Interfaces.
BSD 3-Clause "New" or "Revised" License
9.92k stars 608 forks source link

Visually wide character support #74

Open frizinak opened 8 years ago

frizinak commented 8 years ago

It is currently assumed a rune is exactly 1 column wide. Drawing japanese hiragana for example results in only half of the runes in View.lines to be copied to View.viewLines (visually) because the x offset of each character, as passed to v.setRune, is incremented by 1 instead of 2. Same goes for wrapping and most methods in editor.go.

What does the nul byte in editor.go's writeRune do? It breaks unicode code points that contain that byte, which results in every hiragana character to be (visually) separated with a space after a call to View.Buffer. This should be fixed in View.Buffer and View.ViewBuffer but since I'm not sure of its use, you should double check my 'fix'.

I think most changes should not break existing code that uses gocui except for MoveCursor which now moves entire runes instead of columns. It would be trivial to rename this though and expose the original moveCursor again.

I have only done some really basic testing with demo.go and my own cli-app that uses gocui.

ps: I'm no unicode nor mattn/go-runewidth expert by any means so forgive me if I did something stupid ;)

And thank for your great work on gocui btw.

frizinak commented 8 years ago

forgot to mention moveCursor still needs work wherever v.ox is referenced, if line wrapping is disabled and the origin is shifted, it does not take wide characters into account and overflows by runeWidth - 1

jroimartin commented 8 years ago

First of all, thanks for your contribution! :)

I'm in the process of rewriting the edition mode (issue #60) and I'd prefer to finish it before considering to merge PRs that can be incompatible with this refactoring. Also, this rewrite should fix several issues like the null byte one that you mentioned.

Of course, I'll take into account your PR because I'd definitely like to support variable width runes in the new version.

xh-78 commented 6 years ago

I encountered the same problem, I'm using mattn/go-runewidth as a solution for now. hope this problem can get properly solved.

y-yagi commented 6 years ago

@jroimartin It seems #60 was already closed. How do you feel about this PR?

itchyny commented 6 years ago

@jroimartin Will reconsider including this please? Thanks for your library and it works almost well but missing wide characters support is very critical issue. For example, jesseduffield/lazygit depends on this library and commit messages including wide characters mess up. Thanks.

jesseduffield commented 6 years ago

@itchyny I may as well say this here :)

I'm going to merge this PR into my gocui fork because it seems to do the trick. If it ends up being merged into the main gocui repo down the line I'll revisit this and try to stay at parity with the main repo.

@jroimartin Thankyou for all your effort so far with gocui it's been a massive help to me :)