clippy::same_name_method with Stylize
#1141
Comments
|
The problem is the styled trait is much newer than all the ratatui apps in existence. This would break every file that has ratatui code in every one of them as they'd have to change every call to .style() to .set_style() and add the trait. The size of this break is too large to consider as being a reasonable thing even with deprecation and such. |
|
To move this ahead, I'd suggest thinking through the impact of this change on downstream code. It would be useful to consider the specific magnitude (how much will be affected) and effort involved in updating the code (i.e. specifically what would the compiler do, what find and replace would help, etc.). I've assumed above that these are likely too large to consider, and this might be correct, but perhaps there's an incremental approach that I'm missing here. It might also be worth thinking about possible alternatives to this. Something that I've been thinking about is whether it's worth implementing the stylize methods directly on widgets using either declarative or derive macros rather than a blanket trait implementation. There are likely downsides to this approach too. |
|
Macros would be neat as that can result in rust-analyzer should suggest importing traits for used methods so writing them newly shouldn't be a big problem for people using rust-analyzer. Migrating from direct impl to trait seems like a rather simple change, just add the import. Noting that in the breaking changes section should be easy to adapt to. Also, the Rust error output suggests importing the trait too if I remember correctly. Should be checked. |
I'd be surprised if the performance gains / losses mentioned are meaningful in any reasonable use case. We use macros for the colors and modifiers already, so the maintenance burden isn't high, and the existing implementation is simple and clear on this already. Maintenance burden would be unchanged.
New users regularly have issues copying code and where trait imports are necessary.
The quantity of affected code is relevant here. When you change something that effects every single application built with ratatui like this, think of the time necessary to understand, read, fix, commit, review, and deploy this change for every app. |
Confirmed there is no meaningful difference:
|
Problem
Stylize introduces style and set_style. They are also implemented manually too.
clippy::same_name_methodnotices this.Solution
Probably remove the manually implemented methods and only use the ones implemented on Stylize.
This will be a breaking change as the trait needs to be imported then.
The text was updated successfully, but these errors were encountered: