From 09805e5e863f04da9c140572bb65d0e2816bcc70 Mon Sep 17 00:00:00 2001 From: Tushar Maheshwari Date: Tue, 20 Jul 2021 13:29:47 +0530 Subject: [PATCH] Miscellaneous refactoring (#160) * Reorganize ContainerBase - Reduce Container overloads using default arguments - Extract member function pointers to virtual functions - Separate classes for Vertical, Horizontal and Tab containers * Collect unpack from NodeDecorator subclasses * Reduce redundant expansion for aliases --- examples/component/homescreen.cpp | 29 ++-- .../ftxui/component/screen_interactive.hpp | 1 - include/ftxui/dom/node.hpp | 2 +- src/ftxui/component/container.cpp | 149 +++++++----------- src/ftxui/component/screen_interactive.cpp | 2 - src/ftxui/dom/blink.cpp | 4 +- src/ftxui/dom/bold.cpp | 4 +- src/ftxui/dom/clear_under.cpp | 4 +- src/ftxui/dom/color.cpp | 14 +- src/ftxui/dom/dim.cpp | 4 +- src/ftxui/dom/frame.cpp | 9 +- src/ftxui/dom/inverted.cpp | 4 +- src/ftxui/dom/node_decorator.hpp | 5 +- src/ftxui/dom/underlined.cpp | 4 +- src/ftxui/screen/screen.cpp | 7 +- 15 files changed, 100 insertions(+), 142 deletions(-) diff --git a/examples/component/homescreen.cpp b/examples/component/homescreen.cpp index bd599c5..3e41a42 100644 --- a/examples/component/homescreen.cpp +++ b/examples/component/homescreen.cpp @@ -24,26 +24,19 @@ int main(int argc, const char* argv[]) { int shift = 0; - class Graph { - public: - Graph(int* shift) : shift_(shift) {} - std::vector operator()(int width, int height) { - std::vector output(width); - for (int i = 0; i < width; ++i) { - float v = 0; - v += 0.1f * sin((i + *shift_) * 0.1f); - v += 0.2f * sin((i + *shift_ + 10) * 0.15f); - v += 0.1f * sin((i + *shift_) * 0.03f); - v *= height; - v += 0.5f * height; - output[i] = (int)v; - } - return output; + auto my_graph = [&shift](int width, int height) { + std::vector output(width); + for (int i = 0; i < width; ++i) { + float v = 0.5f; + v += 0.1f * sin((i + shift) * 0.1f); + v += 0.2f * sin((i + shift + 10) * 0.15f); + v += 0.1f * sin((i + shift) * 0.03f); + v *= height; + output[i] = (int)v; } - int* shift_; + return output; }; - Graph my_graph(&shift); auto htop = Renderer([&] { auto frequency = vbox({ text(L"Frequency [Mhz]") | hcenter, @@ -53,7 +46,7 @@ int main(int argc, const char* argv[]) { filler(), text(L"1200 "), filler(), - text(L"0% "), + text(L"0 "), }), graph(std::ref(my_graph)) | flex, }) | flex, diff --git a/include/ftxui/component/screen_interactive.hpp b/include/ftxui/component/screen_interactive.hpp index a6e0b5f..e5b7e4f 100644 --- a/include/ftxui/component/screen_interactive.hpp +++ b/include/ftxui/component/screen_interactive.hpp @@ -24,7 +24,6 @@ class ScreenInteractive : public Screen { static ScreenInteractive FitComponent(); static ScreenInteractive TerminalOutput(); - ~ScreenInteractive(); void Loop(Component); std::function ExitLoopClosure(); diff --git a/include/ftxui/dom/node.hpp b/include/ftxui/dom/node.hpp index f3d03ee..c99e607 100644 --- a/include/ftxui/dom/node.hpp +++ b/include/ftxui/dom/node.hpp @@ -36,7 +36,7 @@ class Node { virtual void Render(Screen& screen); protected: - std::vector children_; + Elements children_; Requirement requirement_; Box box_; }; diff --git a/src/ftxui/component/container.cpp b/src/ftxui/component/container.cpp index 5685fac..45e441e 100644 --- a/src/ftxui/component/container.cpp +++ b/src/ftxui/component/container.cpp @@ -13,44 +13,10 @@ namespace ftxui { class ContainerBase : public ComponentBase { public: - static Component Vertical() { return Vertical({}); } - static Component Vertical(Components children) { - return Vertical(std::move(children), /*selector=*/nullptr); - } - static Component Vertical(Components children, int* selector) { - auto container = std::make_shared(); - container->event_handler_ = &ContainerBase::VerticalEvent; - container->render_handler_ = &ContainerBase::VerticalRender; - container->selector_ = selector ? selector : &container->selected_; + ContainerBase(Components children, int* selector) + : selector_(selector ? selector : &selected_) { for (Component& child : children) - container->Add(std::move(child)); - return container; - } - - static Component Horizontal() { return Horizontal({}); } - static Component Horizontal(Components children) { - return Horizontal(std::move(children), /*selector=*/nullptr); - } - static Component Horizontal(Components children, int* selector) { - auto container = std::make_shared(); - container->event_handler_ = &ContainerBase::HorizontalEvent; - container->render_handler_ = &ContainerBase::HorizontalRender; - container->selector_ = selector ? selector : &container->selected_; - for (Component& child : children) - container->Add(std::move(child)); - return container; - } - - static Component Tab(int* selector) { return Tab({}, selector); } - static Component Tab(Components children, int* selector) { - auto container = std::make_shared(); - container->selector_ = selector ? selector : &container->selected_; - container->event_handler_ = &ContainerBase::TabEvent; - container->render_handler_ = &ContainerBase::TabRender; - container->is_tab_ = true; - for (Component& child : children) - container->Add(std::move(child)); - return container; + Add(std::move(child)); } // Component override. @@ -64,7 +30,7 @@ class ContainerBase : public ComponentBase { if (ActiveChild() && ActiveChild()->OnEvent(event)) return true; - return (this->*event_handler_)(event); + return EventHandler(event); } Component ActiveChild() override { @@ -83,10 +49,36 @@ class ContainerBase : public ComponentBase { } } - private: + protected: // Handlers + virtual bool EventHandler(Event) { return false; } - bool VerticalEvent(Event event) { + virtual bool OnMouseEvent(Event event) { + for (Component& child : children_) { + if (child->OnEvent(event)) + return true; + } + return false; + } + + int selected_ = 0; + int* selector_ = nullptr; +}; + +class VerticalContainer : public ContainerBase { + public: + using ContainerBase::ContainerBase; + + Element Render() override { + Elements elements; + for (auto& it : children_) + elements.push_back(it->Render()); + if (elements.size() == 0) + return text(L"Empty container"); + return vbox(std::move(elements)); + } + + bool EventHandler(Event event) override { int old_selected = *selector_; if (event == Event::ArrowUp || event == Event::Character('k')) (*selector_)--; @@ -100,8 +92,22 @@ class ContainerBase : public ComponentBase { *selector_ = std::max(0, std::min(int(children_.size()) - 1, *selector_)); return old_selected != *selector_; } +}; - bool HorizontalEvent(Event event) { +class HorizontalContainer : public ContainerBase { + public: + using ContainerBase::ContainerBase; + + Element Render() override { + Elements elements; + for (auto& it : children_) + elements.push_back(it->Render()); + if (elements.size() == 0) + return text(L"Empty container"); + return hbox(std::move(elements)); + } + + bool EventHandler(Event event) override { int old_selected = *selector_; if (event == Event::ArrowLeft || event == Event::Character('h')) (*selector_)--; @@ -115,55 +121,22 @@ class ContainerBase : public ComponentBase { *selector_ = std::max(0, std::min(int(children_.size()) - 1, *selector_)); return old_selected != *selector_; } +}; - bool TabEvent(Event) { return false; } +class TabContainer : public ContainerBase { + public: + using ContainerBase::ContainerBase; - bool OnMouseEvent(Event event) { - if (is_tab_) - return ActiveChild()->OnEvent(event); - - for (Component& child : children_) { - if (child->OnEvent(event)) - return true; - } - return false; - } - - using EventHandler = bool (ContainerBase::*)(Event); - using RenderHandler = Element (ContainerBase::*)(); - - Element Render() override { return (this->*render_handler_)(); } - - Element VerticalRender() { - Elements elements; - for (auto& it : children_) - elements.push_back(it->Render()); - if (elements.size() == 0) - return text(L"Empty container"); - return vbox(std::move(elements)); - } - - Element HorizontalRender() { - Elements elements; - for (auto& it : children_) - elements.push_back(it->Render()); - if (elements.size() == 0) - return text(L"Empty container"); - return hbox(std::move(elements)); - } - - Element TabRender() { + Element Render() override { Component active_child = ActiveChild(); if (active_child) return active_child->Render(); return text(L"Empty container"); } - EventHandler event_handler_; - RenderHandler render_handler_; - int selected_ = 0; - int* selector_ = nullptr; - bool is_tab_ = false; + bool OnMouseEvent(Event event) override { + return ActiveChild()->OnEvent(event); + } }; namespace Container { @@ -185,7 +158,7 @@ namespace Container { /// }); /// ``` Component Vertical(Components children) { - return ContainerBase::Vertical(std::move(children)); + return Vertical(std::move(children), nullptr); } /// @brief A list of components, drawn one by one vertically and navigated @@ -207,7 +180,7 @@ Component Vertical(Components children) { /// }); /// ``` Component Vertical(Components children, int* selector) { - return ContainerBase::Vertical(std::move(children), selector); + return std::make_shared(std::move(children), selector); } /// @brief A list of components, drawn one by one horizontally and navigated @@ -228,7 +201,7 @@ Component Vertical(Components children, int* selector) { /// }, &selected_children); /// ``` Component Horizontal(Components children) { - return ContainerBase::Horizontal(std::move(children)); + return Horizontal(std::move(children), nullptr); } /// @brief A list of components, drawn one by one horizontally and navigated @@ -250,14 +223,14 @@ Component Horizontal(Components children) { /// }, selected_children); /// ``` Component Horizontal(Components children, int* selector) { - return ContainerBase::Horizontal(std::move(children), selector); + return std::make_shared(std::move(children), selector); } /// @brief A list of components, where only one is drawn and interacted with at /// a time. The |selector| gives the index of the selected component. This is /// useful to implement tabs. -/// @param selector The index of the drawn children. /// @param children The list of components. +/// @param selector The index of the drawn children. /// @ingroup component /// @see ContainerBase /// @@ -273,7 +246,7 @@ Component Horizontal(Components children, int* selector) { /// }, &tab_drawn); /// ``` Component Tab(Components children, int* selector) { - return ContainerBase::Tab(std::move(children), selector); + return std::make_shared(std::move(children), selector); } } // namespace Container diff --git a/src/ftxui/component/screen_interactive.cpp b/src/ftxui/component/screen_interactive.cpp index 780131e..7d3552c 100644 --- a/src/ftxui/component/screen_interactive.cpp +++ b/src/ftxui/component/screen_interactive.cpp @@ -241,8 +241,6 @@ ScreenInteractive::ScreenInteractive(int dimx, event_sender_ = event_receiver_->MakeSender(); } -ScreenInteractive::~ScreenInteractive() {} - // static ScreenInteractive ScreenInteractive::FixedSize(int dimx, int dimy) { return ScreenInteractive(dimx, dimy, Dimension::Fixed, false); diff --git a/src/ftxui/dom/blink.cpp b/src/ftxui/dom/blink.cpp index 0dbdb01..d426f6e 100644 --- a/src/ftxui/dom/blink.cpp +++ b/src/ftxui/dom/blink.cpp @@ -1,7 +1,7 @@ #include // for make_shared #include // for move -#include "ftxui/dom/elements.hpp" // for Element, unpack, Elements, blink +#include "ftxui/dom/elements.hpp" // for Element, blink #include "ftxui/dom/node.hpp" // for Node #include "ftxui/dom/node_decorator.hpp" // for NodeDecorator #include "ftxui/screen/box.hpp" // for Box @@ -26,7 +26,7 @@ class Blink : public NodeDecorator { /// @brief The text drawn alternates in between visible and hidden. /// @ingroup dom Element blink(Element child) { - return std::make_shared(unpack(std::move(child))); + return std::make_shared(std::move(child)); } } // namespace ftxui diff --git a/src/ftxui/dom/bold.cpp b/src/ftxui/dom/bold.cpp index 7e25de5..cfa3c24 100644 --- a/src/ftxui/dom/bold.cpp +++ b/src/ftxui/dom/bold.cpp @@ -1,7 +1,7 @@ #include // for make_shared #include // for move -#include "ftxui/dom/elements.hpp" // for Element, unpack, Elements, bold +#include "ftxui/dom/elements.hpp" // for Element, bold #include "ftxui/dom/node.hpp" // for Node #include "ftxui/dom/node_decorator.hpp" // for NodeDecorator #include "ftxui/screen/box.hpp" // for Box @@ -26,7 +26,7 @@ class Bold : public NodeDecorator { /// @brief Use a bold font, for elements with more emphasis. /// @ingroup dom Element bold(Element child) { - return std::make_shared(unpack(std::move(child))); + return std::make_shared(std::move(child)); } } // namespace ftxui diff --git a/src/ftxui/dom/clear_under.cpp b/src/ftxui/dom/clear_under.cpp index 188b60f..12501f4 100644 --- a/src/ftxui/dom/clear_under.cpp +++ b/src/ftxui/dom/clear_under.cpp @@ -1,7 +1,7 @@ #include // for make_shared #include // for move -#include "ftxui/dom/elements.hpp" // for Element, unpack, Elements, clear_under +#include "ftxui/dom/elements.hpp" // for Element, clear_under #include "ftxui/dom/node.hpp" // for Node #include "ftxui/dom/node_decorator.hpp" // for NodeDecorator #include "ftxui/screen/box.hpp" // for Box @@ -30,7 +30,7 @@ class ClearUnder : public NodeDecorator { /// @see ftxui::dbox /// @ingroup dom Element clear_under(Element child) { - return std::make_shared(unpack(std::move(child))); + return std::make_shared(std::move(child)); } } // namespace ftxui diff --git a/src/ftxui/dom/color.cpp b/src/ftxui/dom/color.cpp index 297a0a4..af1fbdd 100644 --- a/src/ftxui/dom/color.cpp +++ b/src/ftxui/dom/color.cpp @@ -1,7 +1,7 @@ #include // for make_shared #include // for move -#include "ftxui/dom/elements.hpp" // for Element, unpack, Decorator, Elements, bgcolor, color +#include "ftxui/dom/elements.hpp" // for Element, Decorator, bgcolor, color #include "ftxui/dom/node_decorator.hpp" // for NodeDecorator #include "ftxui/screen/box.hpp" // for Box #include "ftxui/screen/color.hpp" // for Color @@ -11,8 +11,8 @@ namespace ftxui { class BgColor : public NodeDecorator { public: - BgColor(Elements children, Color color) - : NodeDecorator(std::move(children)), color_(color) {} + BgColor(Element child, Color color) + : NodeDecorator(std::move(child)), color_(color) {} void Render(Screen& screen) override { for (int y = box_.y_min; y <= box_.y_max; ++y) { @@ -28,8 +28,8 @@ class BgColor : public NodeDecorator { class FgColor : public NodeDecorator { public: - FgColor(Elements children, Color color) - : NodeDecorator(std::move(children)), color_(color) {} + FgColor(Element child, Color color) + : NodeDecorator(std::move(child)), color_(color) {} void Render(Screen& screen) override { for (int y = box_.y_min; y <= box_.y_max; ++y) { @@ -55,7 +55,7 @@ class FgColor : public NodeDecorator { /// Element document = color(Color::Green, text(L"Success")), /// ``` Element color(Color color, Element child) { - return std::make_shared(unpack(std::move(child)), color); + return std::make_shared(std::move(child), color); } /// @brief Set the background color of an element. @@ -70,7 +70,7 @@ Element color(Color color, Element child) { /// Element document = bgcolor(Color::Green, text(L"Success")), /// ``` Element bgcolor(Color color, Element child) { - return std::make_shared(unpack(std::move(child)), color); + return std::make_shared(std::move(child), color); } /// @brief Decorate using a foreground color. diff --git a/src/ftxui/dom/dim.cpp b/src/ftxui/dom/dim.cpp index 043305f..39b897d 100644 --- a/src/ftxui/dom/dim.cpp +++ b/src/ftxui/dom/dim.cpp @@ -1,7 +1,7 @@ #include // for make_shared #include // for move -#include "ftxui/dom/elements.hpp" // for Element, unpack, Elements, dim +#include "ftxui/dom/elements.hpp" // for Element, dim #include "ftxui/dom/node.hpp" // for Node #include "ftxui/dom/node_decorator.hpp" // for NodeDecorator #include "ftxui/screen/box.hpp" // for Box @@ -26,7 +26,7 @@ class Dim : public NodeDecorator { /// @brief Use a light font, for elements with less emphasis. /// @ingroup dom Element dim(Element child) { - return std::make_shared(unpack(std::move(child))); + return std::make_shared(std::move(child)); } } // namespace ftxui diff --git a/src/ftxui/dom/frame.cpp b/src/ftxui/dom/frame.cpp index 47ce754..86eabff 100644 --- a/src/ftxui/dom/frame.cpp +++ b/src/ftxui/dom/frame.cpp @@ -16,8 +16,7 @@ namespace ftxui { class Select : public Node { public: - Select(std::vector> children) - : Node(std::move(children)) {} + Select(Elements children) : Node(std::move(children)) {} void ComputeRequirement() override { Node::ComputeRequirement(); @@ -31,7 +30,7 @@ class Select : public Node { }; void SetBox(Box box) override { - box_ = box; + Node::SetBox(box); children_[0]->SetBox(box); } }; @@ -44,7 +43,7 @@ Element select(Element child) { class Focus : public Select { public: - Focus(std::vector children) : Select(std::move(children)) {} + using Select::Select; void ComputeRequirement() override { Select::ComputeRequirement(); @@ -85,7 +84,7 @@ Element focus(Element child) { class Frame : public Node { public: - Frame(std::vector children, bool x_frame, bool y_frame) + Frame(Elements children, bool x_frame, bool y_frame) : Node(std::move(children)), x_frame_(x_frame), y_frame_(y_frame) {} void ComputeRequirement() override { diff --git a/src/ftxui/dom/inverted.cpp b/src/ftxui/dom/inverted.cpp index 3f1b4a8..00a67b2 100644 --- a/src/ftxui/dom/inverted.cpp +++ b/src/ftxui/dom/inverted.cpp @@ -1,7 +1,7 @@ #include // for make_shared #include // for move -#include "ftxui/dom/elements.hpp" // for Element, unpack, Elements, inverted +#include "ftxui/dom/elements.hpp" // for Element, inverted #include "ftxui/dom/node.hpp" // for Node #include "ftxui/dom/node_decorator.hpp" // for NodeDecorator #include "ftxui/screen/box.hpp" // for Box @@ -27,7 +27,7 @@ class Inverted : public NodeDecorator { /// colors. /// @ingroup dom Element inverted(Element child) { - return std::make_shared(unpack(std::move(child))); + return std::make_shared(std::move(child)); } } // namespace ftxui diff --git a/src/ftxui/dom/node_decorator.hpp b/src/ftxui/dom/node_decorator.hpp index a1f5709..be62a2c 100644 --- a/src/ftxui/dom/node_decorator.hpp +++ b/src/ftxui/dom/node_decorator.hpp @@ -3,7 +3,8 @@ #include // for move -#include "ftxui/dom/node.hpp" // for Node, Elements +#include "ftxui/dom/elements.hpp" // for Element, unpack +#include "ftxui/dom/node.hpp" // for Node namespace ftxui { struct Box; @@ -11,7 +12,7 @@ struct Box; // Helper class. class NodeDecorator : public Node { public: - NodeDecorator(Elements children) : Node(std::move(children)) {} + NodeDecorator(Element child) : Node(unpack(std::move(child))) {} void ComputeRequirement() override; void SetBox(Box box) override; }; diff --git a/src/ftxui/dom/underlined.cpp b/src/ftxui/dom/underlined.cpp index d75d315..85df371 100644 --- a/src/ftxui/dom/underlined.cpp +++ b/src/ftxui/dom/underlined.cpp @@ -1,7 +1,7 @@ #include // for make_shared #include // for move -#include "ftxui/dom/elements.hpp" // for Element, unpack, Elements, underlined +#include "ftxui/dom/elements.hpp" // for Element, underlined #include "ftxui/dom/node.hpp" // for Node #include "ftxui/dom/node_decorator.hpp" // for NodeDecorator #include "ftxui/screen/box.hpp" // for Box @@ -26,7 +26,7 @@ class Underlined : public NodeDecorator { /// @brief Make the underlined element to be underlined. /// @ingroup dom Element underlined(Element child) { - return std::make_shared(unpack(std::move(child))); + return std::make_shared(std::move(child)); } } // namespace ftxui diff --git a/src/ftxui/screen/screen.cpp b/src/ftxui/screen/screen.cpp index 92a76f6..65ae09f 100644 --- a/src/ftxui/screen/screen.cpp +++ b/src/ftxui/screen/screen.cpp @@ -38,11 +38,6 @@ static const char* MOVE_LEFT = "\r"; static const char* MOVE_UP = "\x1B[1A"; static const char* CLEAR_LINE = "\x1B[2K"; -bool In(const Box& stencil, int x, int y) { - return stencil.x_min <= x && x <= stencil.x_max && // - stencil.y_min <= y && y <= stencil.y_max; -} - Pixel dev_null_pixel; #if defined(_WIN32) @@ -195,7 +190,7 @@ wchar_t& Screen::at(int x, int y) { /// @param x The pixel position along the x-axis. /// @param y The pixel position along the y-axis. Pixel& Screen::PixelAt(int x, int y) { - return In(stencil, x, y) ? pixels_[y][x] : dev_null_pixel; + return stencil.Contain(x, y) ? pixels_[y][x] : dev_null_pixel; } /// @brief Return a string to be printed in order to reset the cursor position