-
-
Notifications
You must be signed in to change notification settings - Fork 376
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(cell): add EMPTY and (const) new method #1143
Conversation
This simplifies calls to `Buffer::filled` in tests
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1143 +/- ##
=======================================
- Coverage 94.2% 94.2% -0.1%
=======================================
Files 60 60
Lines 14623 14511 -112
=======================================
- Hits 13780 13672 -108
+ Misses 843 839 -4 ☔ View full report in Codecov by Sentry. |
A (breaking) follow up to this will be the following performance improvement (removes one needless clone / drop): /// Returns a Buffer with all cells initialized with the attributes of the given Cell
#[must_use]
- pub fn filled(area: Rect, cell: &Cell) -> Self {
+ pub fn filled(area: Rect, cell: Cell) -> Self {
let size = area.area() as usize;
- let content = vec![cell.clone(); size];
+ let content = vec![cell; size];
Self { area, content }
} As this would result in a bunch of conflicts (and is breaking) I'll wait with that until this one is sorted out. |
+1 to this - looks like a good idea. It's breaking but in a function that's only really useful for testing, so that's not too bad. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another idea that springs from this - doing a quick search suggests perhaps a Cell::styled()
constructor could be useful
// instead of:
buf.get_mut(x,y)
.set_symbol("x")
.set_style(style);
// what about:
buf[(x,y)] = Cell::styled("x", style);
I'm not sure what impact this would have on allocations (likely many) / perf (likely little). Doesn't have to be part of this PR at all, but I'm curious to hear your thoughts on this.
The constructor in this PR is mainly simplifying tests. |
This simplifies calls to `Buffer::filled` in tests.
This simplifies calls to
Buffer::filled
in tests.