Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(border): add border! macro for easy bitflag manipulation #11

Merged
merged 12 commits into from
May 21, 2023

Conversation

orhun
Copy link
Member

@orhun orhun commented Feb 12, 2023

Upstream: #484

Description

I have added the border! macro. It can be called with any combination of TOP, BOTTOM, LEFT, RIGHT, NONE, or ALL to return a widgets::Borders object with the corrosponding flags set. This is helpful because the current methods of creating custom Borders is ugly and long. Further things such as Borders.add() or Borders.remove() do not allowing chaining which makes constructing the border you want either a multiline operation or requires oring Borders objects. This macro is cleaner, shorter, and much more readable.
Compare:
This is a use of Borders from the widgets::Block documentation
Block::default() .title("Block") .borders(Borders::LEFT | Borders::RIGHT) .border_style(Style::default().fg(Color::White)) .border_type(BorderType::Rounded) .style(Style::default().bg(Color::Black));

This is the same code with the border! macro
Block::default() .title("Block") .borders(border!(LEFT, RIGHT)) .border_style(Style::default().fg(Color::White)) .border_type(BorderType::Rounded) .style(Style::default().bg(Color::Black));

As you can see the second is much easier to read and understand. This is a fairly trivial example but if you need a border with three sides the differences becomes much clearer. border!(TOP, LEFT, RIGHT) instead of Borders::TOP | Borders::LEFT | Borders::RIGHT.

The macro also provides a syntactic sugar for Borders::NONE. Instead of calling border!(NONE) or using Borders::NONE directly you can just call border!() which return Borders::NONE.

Interally the macro functions by calling Borders::empty() and then using .insert() to insert each specified flag. If no flags are specified the macro directly returns Borders::NONE and doesn't waste effort creating an empty Borders and inserting.

I have done my best to add inline documentation to the addition but I'm unsure if I did it correctly/formatted it correctly. it currently passes cargo test but a quick glance over it would be appreciated.

Testing guidelines

I have added tests for the macro to tests in border_macro.rs. These include tests for ALL, specific sides, and the syntactic sugar for NONE: border!(). All tests pass on debian linux when running cargo test.

Checklist

border! macro that takes TOP, BOTTOM, LEFT, RIGHT, and ALL and returns a Borders object.
Added tests for the border! macro.
split tests up to make it easier to see why it's failing.
Fixed the treatment of border!() so that it returns Borders::NONE instead of the incorrect Borders::empty(). I also changed from the incorrect Borders::all() to Borders::ALL.
semicolon in examples
added needed import to example
needed to add another import
removed some imports that weren't needed in the new tests
@mindoodoo mindoodoo added the Type: Enhancement New feature or request label Feb 14, 2023
Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@joshka joshka added the Type: Imported from tui-rs This issue / PR was Imported from tui-rs - kept for posterity / ease of finding historical ideas label May 5, 2023
@joshka joshka changed the title border! macro for the easy and pretty creation of Borders bitflags. feat(border): add border! macro for easy bitflag manipulation May 7, 2023
@sayanarijit
Copy link
Member

I think we discussed earlier to avoid proc macros. Similar to macros for borders, it can be also used for style, constraints and entire widgets. But the more macros we add the worse it'll get. They are slow to compile and difficult to debug. cc @orhun @mindoodoo

@joshka
Copy link
Member

joshka commented May 21, 2023

I think we discussed earlier to avoid proc macros. Similar to macros for borders, it can be also used for style, constraints and entire widgets. But the more macros we add the worse it'll get. They are slow to compile and difficult to debug. cc @orhun @mindoodoo

That's a fair call. Perhaps we suggest this ends up either feature flagged or in a separate lib?

@sayanarijit
Copy link
Member

Feature flag sounds good.

@orhun
Copy link
Member Author

orhun commented May 21, 2023

Added a feature flag called macros and added this functionality behind it.

@orhun orhun requested a review from joshka May 21, 2023 14:56
@joshka joshka merged commit ef8bc7c into main May 21, 2023
@joshka joshka deleted the 484/master branch May 21, 2023 22:26
@joshka
Copy link
Member

joshka commented May 21, 2023

Merged with description:

Adds a border! macro that takes TOP, BOTTOM, LEFT, RIGHT, and ALL and
returns a Borders object. An empty border!() call returns
Borders::NONE

This is gated behind a macros feature flag to ensure short build
times. To enable, add the following to your Cargo.toml:

ratatui = { version = 0.21.0, features = ["macros"] }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request Type: Imported from tui-rs This issue / PR was Imported from tui-rs - kept for posterity / ease of finding historical ideas
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants