ArthurSonzogni / FTXUI

:computer: C++ Functional Terminal User Interface. :heart:
MIT License
6.64k stars 399 forks source link

Crash when input cjk character [linux] #2

Closed codehz closed 5 years ago

codehz commented 5 years ago

image LANG=zh_CN.utf-8 LC_ALL=zh_CN.utf-8

https://github.com/ArthurSonzogni/FTXUI/blob/master/src/ftxui/screen/string.cpp#L8

ArthurSonzogni commented 5 years ago

Thanks for opening this issue!

I still need to implement properly: Event GetEvent() in: https://github.com/ArthurSonzogni/FTXUI/blob/master/src/ftxui/component/screen_interactive.cpp It haven't seen any good document on how to read terminal input correctly.

Pull request are welcome if you want. Otherwise, I will try to fix this myself.

ArthurSonzogni commented 5 years ago

I made https://github.com/ArthurSonzogni/FTXUI/commit/001a0ae92579da947cc5d8fc2fb1143aa5336cc9 to fix this issue.

Could you confirm this fixed your issue?

codehz commented 5 years ago

@ArthurSonzogni image not crash now, but the layout is wrong.. example text: (you can just copy and paste it) 测试

PS: most of CJK character is full-width character(=2 normal character), so it need to be specifically implemented. https://gist.github.com/parisk/71b4631bffbe078b77e7e2e2bd6a84c5 it is an example to measure the character width

ArthurSonzogni commented 5 years ago

I see. It is not going to be easy, but making a fix seems doable. Thanks! I will use your example functions.

ArthurSonzogni commented 5 years ago

This should be fixed now! I used the functions you provided (thanks!) I am a bit surprised, I just had to include your functions and then use them in a 3-4 lines changes.

Please let me know whether everything is now perfect or not.

I purposedly let a glitch. See the expectations of TextTest.CJK_2.

// See https://github.com/ArthurSonzogni/FTXUI/issues/2#issuecomment-504871456
TEST(TextTest, CJK) {
  auto element = text(L"测试") | border;
  Screen screen(6, 3);
  Render(screen, element.get());
  EXPECT_EQ(
      "┌────┐\n"
      "│测试│\n"
      "└────┘",
      screen.ToString());
}

// See https://github.com/ArthurSonzogni/FTXUI/issues/2#issuecomment-504871456
TEST(TextTest, CJK_2) {
  auto element = text(L"测试") | border;
  Screen screen(5, 3);
  Render(screen, element.get());
  EXPECT_EQ(
      "┌───┐\n"
      "│测试\n"
      "└───┘",
      screen.ToString());
}

// See https://github.com/ArthurSonzogni/FTXUI/issues/2#issuecomment-504871456
TEST(TextTest, CJK_3) {
  auto element = text(L"测试") | border;
  Screen screen(4, 3);
  Render(screen, element.get());
  EXPECT_EQ(
      "┌──┐\n"
      "│测│\n"
      "└──┘",
      screen.ToString());
}

When the text doesn't fit the allocated space, I let the last 2-width character to be displayed, which may be drawn outside of the allocated space. It looks good enough, but maybe I should reconsider this in the future.

One question: I haven't understood whether I should use mk_wcswidth ou mk_wcswidth_cjk. Do you know which one is the correct one in my case?

codehz commented 5 years ago

I have tested both input and output, it works!

About the mk_wcswidth_cjk: I think this is caused by historical reasons. Some terminal emulators will display some ambiguous symbols as full width, but others will not. In the latest unicode standard, those characters are called "East Asian Ambiguous" characters. ( https://unicode.org/reports/tr11/ )

reference: https://github.com/xtermjs/xterm.js/issues/1453 https://github.com/mintty/mintty/issues/88 https://superuser.com/questions/573876/how-to-let-gnome-terminal-to-use-specific-font-to-display-punctuations-in-their http://vimdoc.sourceforge.net/htmldoc/options.html#'ambiwidth'

you can search "ambiguous full width" in google for details..

PS: I suggest to add an option to switch between two algorithms just like vim, xterm.js and gnome terminal

codehz commented 5 years ago

there is an IME issue: image most of CJK user use IME (Input Method) to type cjk character(you know, it is impossible to type them directly...) there need a area called "preview area" to choose which character to insert. and now there is a problem, the "fake cursor" is not a real cursor. and the preview text will be append to the end.

Yes, you cannot get the preview text in terminal program, it just rendered by terminal emulator, but you can move it (by move the **real** cursor, even use *invisible* cursor. In some terminal emulator, *invisible* cursor will make the preview text disappear, but it's fine, because the IME has both preview text and float window)

(some IME only use float window to preview character, the float window is also need to attach to the cursor position)

it is a screenshot from whiptail image Hide the cursor will cause: (the cursor is attached to the focused control) image

possible fix:

  1. just move the real cursor to fake cursor's position, and hide it
  2. use real cursor instead of fake cursor
ArthurSonzogni commented 5 years ago

Thanks you so much for your explanation!

I didn't manage to install any IME on my linux laptop. I tried ibus-anthy, but wasn't able to show the preview character box.

Anyway, I think I fixed the cursor problem. In theory, it should work. Please let me know if this fix the problem.

codehz commented 5 years ago

@ArthurSonzogni Yes, it fixed .... almostly the problem is that the cursor is at the end of the input box when the input box is empty..

ArthurSonzogni commented 5 years ago

I see. Thanks! When the ftxui::component::input content size is zero. It doesn't emit any ftxui::dom::focus element.

I just added a fix.

codehz commented 5 years ago

Confirmed, fixed finally!