Skip to content
New issue

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

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

Already on GitHub? # to your account

perf(block): use Block::bordered #1041

Merged
merged 4 commits into from
May 2, 2024
Merged

perf(block): use Block::bordered #1041

merged 4 commits into from
May 2, 2024

Conversation

EdJoPaTo
Copy link
Contributor

Block::bordered() is shorter than Block::new().borders(Borders::ALL), requires one less import (Borders) and in case Block::default() was used before can even be const.

This mostly changes examples, documentation, and tests.
As I used a lot of regular expressions I could have overlooked something when checking all the changes myself. Another set of eyes might find something I missed.

Copy link

codecov bot commented Apr 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.4%. Comparing base (64eb391) to head (5c33e29).

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1041     +/-   ##
=======================================
- Coverage   89.4%   89.4%   -0.1%     
=======================================
  Files         61      61             
  Lines      15468   15440     -28     
=======================================
- Hits       13839   13808     -31     
- Misses      1629    1632      +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orhun
Copy link
Member

orhun commented Apr 16, 2024

There seems to be some usages left in README.md, chart example and other places. I can fix it if you want 🐻

@EdJoPaTo
Copy link
Contributor Author

README, good call! I only checked *.rs files to prevent CHANGELOG.md mishaps 😇

And… Borders::all()… havnt thought of that while searching the repo.

I think I got them all now?

Copy link
Member

@orhun orhun left a comment

Choose a reason for hiding this comment

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

LGTM

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 other than my comments.

@joshka
Copy link
Member

joshka commented May 2, 2024

Thanks for keeping the block tests in. LGTM

@joshka joshka merged commit 366c2a0 into main May 2, 2024
33 checks passed
@joshka joshka deleted the block-bordered branch May 2, 2024 10:09
joshka pushed a commit to nowNick/ratatui that referenced this pull request May 24, 2024
`Block::bordered()` is shorter than
`Block::new().borders(Borders::ALL)`, requires one less import
(`Borders`) and in case `Block::default()` was used before can even be
`const`.
joshka pushed a commit to erak/ratatui that referenced this pull request Oct 14, 2024
`Block::bordered()` is shorter than
`Block::new().borders(Borders::ALL)`, requires one less import
(`Borders`) and in case `Block::default()` was used before can even be
`const`.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants