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

perf(layout): const functions #951

Merged
merged 1 commit into from Feb 14, 2024
Merged

Conversation

EdJoPaTo
Copy link
Member

The layout can often contain things that are already known at compile time which speeds up things on runtime. Especially as render is called a lot.

With 0.26.0 the Rect::contains method is now accepting a Position. While contains is const, Position::new is not which triggered the creation of this PR. I switched to Position { x, y }, but the compiler should be able to do the same in const with ::new().

I added the lint to the layout module, but I don't really know what other plans of this project are in regard to configuring lints. I could also just remove the lint there, but mistakes like the current one will happen again as the code uses a lot of const already. Do you have other thoughts on how to configure the lints best?

I observed the missing_const_for_fn clippy lint in other areas of the library too, but it's mostly pointless there. Some Widgets and WidgetStates could get ::new() implementations which are const which would also allow for optimizations there.

Personally I also think that more clippy::pedantic and clippy::nursery lints could keep the code quality of ratatui consistent. Like in the current case many things are already const, but they are forgotten in other places as no lints are ensuring that.

@joshka
Copy link
Member

joshka commented Feb 14, 2024

Personally I also think that more clippy::pedantic and clippy::nursery lints could keep the code quality of ratatui consistent. Like in the current case many things are already const, but they are forgotten in other places as no lints are ensuring that.

+1 on these - the issue is that turning them on makes a lot of noise initially, so it's a bunch of work drilling down on the lint errors. Once it's in though things are really nice.

@joshka joshka merged commit 11b452d into ratatui-org:main Feb 14, 2024
30 of 31 checks passed
@EdJoPaTo EdJoPaTo deleted the const-layout branch February 14, 2024 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants