From 81b428af5a5613368567bf192f79627e19e332d7 Mon Sep 17 00:00:00 2001 From: Arthur Sonzogni Date: Sun, 4 Jul 2021 17:38:31 +0200 Subject: [PATCH] Implement Fallback for microsoft's terminals. (#138) I finally got access to a computer using the Microsoft's Windows OS. That's the opportunity to find and mitigate all the problems encountered. This patch: 1. Introduce an option and a C++ definition to enable fallback for Microsoft's terminal emulators. This allows me to see/test the Microsoft output from Linux. This also allows Windows users to remove the fallback and target non Microsoft terminals on Windows if needed. 2. Microsoft's terminal suffer from a race condition bug when reporting the cursor position: https://github.com/microsoft/terminal/pull/7583. The mitigation is not to ask for the cursor position in fullscreen mode where it isn't really needed and request it less often. This fixes: https://github.com/ArthurSonzogni/FTXUI/issues/136 3. Microsoft's terminal do not handle properly hidding the cursor. Instead the character under the cursor is hidden, which is a big problem. As a result, we don't enable setting the cursor to the best position for [input method editors](https://en.wikipedia.org/wiki/Input_method), It will be displayed at the bottom right corner. See: - https://github.com/microsoft/terminal/issues/1203 - https://github.com/microsoft/terminal/issues/3093 4. Microsoft's terminals do not provide a way to query if they support colors. As a fallback, assume true colors is supported. See issue: - https://github.com/microsoft/terminal/issues/1040 This mitigates: - https://github.com/ArthurSonzogni/FTXUI/issues/135 5. The "cmd" on Windows do not properly report its dimension. Powershell works correctly. As a fallback, use a 80x80 size instead of 0x0. 6. There are several dom elements and component displayed incorrectly, because the font used is missing several unicode glyph. Use alternatives or less detailled one as a fallback. --- CMakeLists.txt | 11 ++++++++++ cmake/ftxui_set_options.cmake | 6 +++++ include/ftxui/component/checkbox.hpp | 5 ----- include/ftxui/component/radiobox.hpp | 5 ----- .../ftxui/component/screen_interactive.hpp | 4 ++-- src/ftxui/component/checkbox.cpp | 11 +++++++++- src/ftxui/component/component_test.cpp | 2 +- src/ftxui/component/radiobox.cpp | 11 +++++++++- src/ftxui/component/screen_interactive.cpp | 21 +++++++++++++++--- src/ftxui/dom/frame.cpp | 20 +++++++++++++++++ src/ftxui/dom/gauge.cpp | 6 +++++ src/ftxui/dom/graph.cpp | 6 +++++ src/ftxui/screen/terminal.cpp | 22 ++++++++++++++----- 13 files changed, 107 insertions(+), 23 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1e24ea9c6..fcd5bf6ad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,6 +13,17 @@ option(FTXUI_BUILD_TESTS "Set to ON to build tests" OFF) option(FTXUI_BUILD_TESTS_FUZZER "Set to ON to enable fuzzing" OFF) option(FTXUI_ENABLE_INSTALL "Generate the install target" ON) +set(FTXUI_MICROSOFT_TERMINAL_FALLBACK_HELP_TEXT "On windows, assume the \ +terminal used will be one of Microsoft and use a set of reasonnable fallback \ +to counteract its implementations problems.") +if (WIN32) + option(FTXUI_MICROSOFT_TERMINAL_FALLBACK + ${FTXUI_MICROSOFT_TERMINAL_FALLBACK_HELP_TEXT} ON) +else() + option(FTXUI_MICROSOFT_TERMINAL_FALLBACK + ${FTXUI_MICROSOFT_TERMINAL_FALLBACK_HELP_TEXT} OFF) +endif() + add_library(screen STATIC src/ftxui/screen/box.cpp src/ftxui/screen/color.cpp diff --git a/cmake/ftxui_set_options.cmake b/cmake/ftxui_set_options.cmake index a25197fd0..682138c74 100644 --- a/cmake/ftxui_set_options.cmake +++ b/cmake/ftxui_set_options.cmake @@ -36,6 +36,11 @@ function(ftxui_set_options library) target_compile_options(${library} PRIVATE "-Wdeprecated") target_compile_options(${library} PRIVATE "-Wshadow") endif() + + if (FTXUI_MICROSOFT_TERMINAL_FALLBACK) + target_compile_definitions(${library} + PRIVATE "FTXUI_MICROSOFT_TERMINAL_FALLBACK") + endif() endfunction() if (EMSCRIPTEN) @@ -44,3 +49,4 @@ if (EMSCRIPTEN) string(APPEND CMAKE_CXX_FLAGS " -s USE_PTHREADS") string(APPEND CMAKE_CXX_FLAGS " -s PROXY_TO_PTHREAD") endif() + diff --git a/include/ftxui/component/checkbox.hpp b/include/ftxui/component/checkbox.hpp index 9cd1db45f..d16cef33d 100644 --- a/include/ftxui/component/checkbox.hpp +++ b/include/ftxui/component/checkbox.hpp @@ -25,13 +25,8 @@ class CheckboxBase : public ComponentBase { CheckboxBase(ConstStringRef label, bool* state); ~CheckboxBase() override = default; -#if defined(_WIN32) - std::wstring checked = L"[X] "; /// Prefix for a "checked" state. - std::wstring unchecked = L"[ ] "; /// Prefix for an "unchecked" state. -#else std::wstring checked = L"▣ "; /// Prefix for a "checked" state. std::wstring unchecked = L"☐ "; /// Prefix for a "unchecked" state. -#endif Decorator focused_style = inverted; /// Decorator used when focused. Decorator unfocused_style = nothing; /// Decorator used when unfocused. diff --git a/include/ftxui/component/radiobox.hpp b/include/ftxui/component/radiobox.hpp index 8df7a32c2..9614f95fb 100644 --- a/include/ftxui/component/radiobox.hpp +++ b/include/ftxui/component/radiobox.hpp @@ -27,13 +27,8 @@ class RadioboxBase : public ComponentBase { int focused = 0; -#if defined(_WIN32) - std::wstring checked = L"(*) "; - std::wstring unchecked = L"( ) "; -#else std::wstring checked = L"◉ "; std::wstring unchecked = L"○ "; -#endif Decorator focused_style = inverted; Decorator unfocused_style = nothing; diff --git a/include/ftxui/component/screen_interactive.hpp b/include/ftxui/component/screen_interactive.hpp index 827602f1e..9cb15a167 100644 --- a/include/ftxui/component/screen_interactive.hpp +++ b/include/ftxui/component/screen_interactive.hpp @@ -56,8 +56,8 @@ class ScreenInteractive : public Screen { std::atomic quit_ = false; - int cursor_x_ = 0; - int cursor_y_ = 0; + int cursor_x_ = 1; + int cursor_y_ = 1; bool mouse_captured = false; }; diff --git a/src/ftxui/component/checkbox.cpp b/src/ftxui/component/checkbox.cpp index f3cd1c598..8d24d435c 100644 --- a/src/ftxui/component/checkbox.cpp +++ b/src/ftxui/component/checkbox.cpp @@ -40,7 +40,16 @@ CheckboxBase* CheckboxBase::From(Component component) { } CheckboxBase::CheckboxBase(ConstStringRef label, bool* state) - : label_(label), state_(state) {} + : label_(label), state_(state) { +#if defined(FTXUI_MICROSOFT_TERMINAL_FALLBACK) + // Microsoft terminal do not use fonts able to render properly the default + // radiobox glyph. + if (checked == L"▣ ") + checked = L"[X]"; + if (unchecked == L"☐ ") + unchecked = L"[ ]"; +#endif +} Element CheckboxBase::Render() { bool is_focused = Focused(); diff --git a/src/ftxui/component/component_test.cpp b/src/ftxui/component/component_test.cpp index cfa46d71f..5e1813c0c 100644 --- a/src/ftxui/component/component_test.cpp +++ b/src/ftxui/component/component_test.cpp @@ -1,8 +1,8 @@ -#include // for Test, SuiteApiResolver, TestInfo (ptr only), TEST, TestFactoryImpl #include // for shared_ptr, allocator, make_shared, __shared_ptr_access #include "ftxui/component/captured_mouse.hpp" // for ftxui #include "ftxui/component/component_base.hpp" // for ComponentBase, Component +#include "gtest/gtest_pred_impl.h" // for Test, SuiteApiResolver, TEST, TestFactoryImpl using namespace ftxui; diff --git a/src/ftxui/component/radiobox.cpp b/src/ftxui/component/radiobox.cpp index 83730c59f..b721e2d0f 100644 --- a/src/ftxui/component/radiobox.cpp +++ b/src/ftxui/component/radiobox.cpp @@ -50,7 +50,16 @@ RadioboxBase* RadioboxBase::From(Component component) { RadioboxBase::RadioboxBase(const std::vector* entries, int* selected) - : entries_(entries), selected_(selected) {} + : entries_(entries), selected_(selected) { +#if defined(FTXUI_MICROSOFT_TERMINAL_FALLBACK) + // Microsoft terminal do not use fonts able to render properly the default + // radiobox glyph. + if (checked == L"◉ ") + checked = L"(*)"; + if (unchecked == L"○ ") + unchecked = L"( )"; +#endif +} Element RadioboxBase::Render() { std::vector elements; diff --git a/src/ftxui/component/screen_interactive.cpp b/src/ftxui/component/screen_interactive.cpp index 839bb98d3..780131ee1 100644 --- a/src/ftxui/component/screen_interactive.cpp +++ b/src/ftxui/component/screen_interactive.cpp @@ -450,10 +450,25 @@ void ScreenInteractive::Draw(Component component) { cursor_.y = dimy_ - 1; } - static int i = -2; - if (i % 10 == 0) - std::cout << DeviceStatusReport(DSRMode::kCursor); + // Periodically request the terminal emulator the frame position relative to + // the screen. This is useful for converting mouse position reported in + // screen's coordinates to frame's coordinates. + static constexpr int cursor_refresh_rate = +#if defined(FTXUI_MICROSOFT_TERMINAL_FALLBACK) + // Microsoft's terminal suffers from a [bug]. When reporting the cursor + // position, several output sequences are mixed together into garbage. + // This causes FTXUI user to see some "1;1;R" sequences into the Input + // component. See [issue]. Solution is to request cursor position less + // often. [bug]: https://github.com/microsoft/terminal/pull/7583 [issue]: + // https://github.com/ArthurSonzogni/FTXUI/issues/136 + 150; +#else + 20; +#endif + static int i = -3; ++i; + if (!use_alternative_screen_ && (i % cursor_refresh_rate == 0)) + std::cout << DeviceStatusReport(DSRMode::kCursor); Render(*this, document); diff --git a/src/ftxui/dom/frame.cpp b/src/ftxui/dom/frame.cpp index ba815f906..47ce75410 100644 --- a/src/ftxui/dom/frame.cpp +++ b/src/ftxui/dom/frame.cpp @@ -53,7 +53,27 @@ class Focus : public Select { void Render(Screen& screen) override { Select::Render(screen); + + // Setting the cursor to the right position allow folks using CJK (China, + // Japanese, Korean, ...) characters to see their [input method editor] + // displayed at the right location. See [issue]. + // + // [input method editor]: + // https://en.wikipedia.org/wiki/Input_method + // + // [issue]: + // https://github.com/ArthurSonzogni/FTXUI/issues/2#issuecomment-505282355 + // + // Unfortunately, Microsoft terminal do not handle properly hidding the + // cursor. Instead the character under the cursor is hidden, which is a big + // problem. As a result, we can't enable setting cursor to the right + // location. It will be displayed at the bottom right corner. + // See: + // https://github.com/microsoft/terminal/issues/1203 + // https://github.com/microsoft/terminal/issues/3093 +#if !defined(FTXUI_MICROSOFT_TERMINAL_FALLBACK) screen.SetCursor(Screen::Cursor{box_.x_min, box_.y_min}); +#endif } }; diff --git a/src/ftxui/dom/gauge.cpp b/src/ftxui/dom/gauge.cpp index d955d983f..3c2888d62 100644 --- a/src/ftxui/dom/gauge.cpp +++ b/src/ftxui/dom/gauge.cpp @@ -8,7 +8,13 @@ namespace ftxui { +#if defined(FTXUI_MICROSOFT_TERMINAL_FALLBACK) +// Microsoft's terminals often use fonts not handling the 8 unicode characters +// for representing the whole gauge. Fallback with less. +static wchar_t charset[] = L" ▌▌▌███"; +#else static wchar_t charset[] = L" ▏▎▍▌▋▊▉█"; +#endif class Gauge : public Node { public: diff --git a/src/ftxui/dom/graph.cpp b/src/ftxui/dom/graph.cpp index 4261bdefc..a1faa65e9 100644 --- a/src/ftxui/dom/graph.cpp +++ b/src/ftxui/dom/graph.cpp @@ -10,7 +10,13 @@ namespace ftxui { +// Microsoft's terminals often use fonts not handling the 8 unicode characters +// for representing the whole gauge. Fallback with less. +#if defined(FTXUI_MICROSOFT_TERMINAL_FALLBACK) +const wchar_t charset[] = L" █ █████"; +#else const wchar_t charset[] = L" ▗▐▖▄▟▌▙█"; +#endif class Graph : public Node { public: diff --git a/src/ftxui/screen/terminal.cpp b/src/ftxui/screen/terminal.cpp index 213377f41..136843975 100644 --- a/src/ftxui/screen/terminal.cpp +++ b/src/ftxui/screen/terminal.cpp @@ -23,12 +23,15 @@ Terminal::Dimensions Terminal::Size() { return Dimensions{140, 43}; #elif defined(_WIN32) CONSOLE_SCREEN_BUFFER_INFO csbi; - int columns, rows; - GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &csbi); - columns = csbi.srWindow.Right - csbi.srWindow.Left + 1; - rows = csbi.srWindow.Bottom - csbi.srWindow.Top + 1; - return Dimensions{columns, rows}; + if (GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &csbi)) { + return Dimensions{csbi.srWindow.Right - csbi.srWindow.Left + 1, + csbi.srWindow.Bottom - csbi.srWindow.Top + 1}; + } + + // The Microsoft default "cmd" returns errors above. + return Dimensions{80, 80}; + #else winsize w; ioctl(STDOUT_FILENO, TIOCGWINSZ, &w); @@ -61,6 +64,15 @@ Terminal::Color ComputeColorSupport() { if (Contains(COLORTERM, "256") || Contains(TERM, "256")) return Terminal::Color::Palette256; +#if defined(FTXUI_MICROSOFT_TERMINAL_FALLBACK) + // Microsoft terminals do not properly declare themselve supporting true + // colors: https://github.com/microsoft/terminal/issues/1040 + // As a fallback, assume microsoft terminal are the ones not setting those + // variables, and enable true colors. + if (TERM == "" && COLORTERM == "") + return Terminal::Color::TrueColor; +#endif + return Terminal::Color::Palette16; }