Feedback about the template.
ArthurSonzogni opened this issue · 1 comments
Hello,
I just tried the template. Things are working correctly!
I only had two issues build. It blocked me 10min.
- I installed
conan
and got weird errors. They were really not easy to diagnose. The fix was to makepython3
the default alternative instead ofpython2.7
. clang-tidy
is a "required-dependency". It might worth mentioning it in:README_dependencies.md
or making it work without it.
Some comments:
-
Bitmap::Render writes into the [0, width-1] x [0, height-1] box. However you might get an area smaller than what was asked in
ComputeRequirement()
.
This would normally result in out of bounds errors.
Fortunately, FTXUI has the concept of a "stencil", inhibiting writing outside of the box assigned to you:
https://github.com/ArthurSonzogni/FTXUI/blob/548fa51b7115551b42c0f12235de9eb79e7e33e3/src/ftxui/screen/screen.cpp#L408-L413
So this works, despite this. Not sure if you knew it. -
I see
Bitmap::SetBox
overridesNode::SetBox
, but provide the same implementation has the default one. You might want to remove this line if this wasn't added purposefully to help people to learn it. -
spdlog
is included, but I don't see any usage. I am not sure how this will interact with FTXUI, if used. -
You can replace:
auto container = ftxui::Container::Horizontal(all_buttons);
auto renderer = ftxui::Renderer(container, make_layout);
by
auto renderer = Renderer(make_layout);
The Renderer
function "decorates" a component, by replacing it's "Render()" function implementation, by the one provided. This is a alternative to using c++ composition/inheritance, that is sometimes quicker to write.
In your case, the decorated component doesn't do much. So the second version taking only the function rendering the interface can be used.
Thank you for the insight @ArthurSonzogni !
- I guess I'll leave it how it is, since it's safe at least for now, and I'm not sure how I would deal with it otherwise. (scaling?)
- Fixed
- Very good point, which I wondered about. I'll leave it for now, because developers can still sink to files if they choose to, or sink to a window inside of FTXUI
- Fixed - I need a better understanding of how buttons and button order interacts with layout.
See: #8