refactor: apply a bunch of lints

[EdJoPaTo]

In the beginning I wanted to add const improvements to widgets I use, but then I thought I should apply a bunch of lints first before doing more refacting like const new methods.
Applying them to the whole project is similar in comparison to just a few files, so I did that.

I ran my personal bunch of lints over the entire project (pedantic, nursery and some restricted) and applied some of them. I went through the lints and checked what is at least partially done already in this repo to ensure a consistent state. For example use_self is partially used in this repo. The lint ensures its done everywhere consistently. The same with the other lints.
Currently, lints which did not trigger at least some kind of refactoring are not added. In the long run I would like to enable clippy::pedantic (except lints like module_name_repetitions) for this project.

This creates a lot of changes but except the change to the WidgetRef it should not be a breaking change. (It somewhat amazed me that the WidgetRef API required ownership and not a reference or have I overlooked something? It's in its own commit)

This probably creates some churn with other PR created before this is merged.

Looking at all the changes together is somewhat pointless. Each commit should be checked on its own. It should also be considered if this should be squashed on merge like the other PR do normally. I think it's useful to keep the commits individually. But that is not my decision.

I can rebase this and add/improve/remove commits/lints. I can also cherry-pick some into their own PR to fast-lane them. We should discuss the lints.

[codecov]

Codecov Report

Attention: Patch coverage is 91.57706% with 47 lines in your changes are missing coverage. Please review.

Project coverage is 91.9%. Comparing base (c12bcfe) to head (1c2e86a).

Files	Patch %	Lines
src/backend/crossterm.rs	38.8%	22 Missing ⚠️
src/backend/termwiz.rs	77.5%	9 Missing ⚠️
src/style/palette/material.rs	0.0%	4 Missing ⚠️
src/widgets/sparkline.rs	80.0%	2 Missing ⚠️
src/widgets/tabs.rs	80.0%	2 Missing ⚠️
src/buffer/buffer.rs	88.8%	1 Missing ⚠️
src/layout/constraint.rs	96.7%	1 Missing ⚠️
src/layout/rect.rs	94.1%	1 Missing ⚠️
src/style/color.rs	97.4%	1 Missing ⚠️
src/widgets/barchart.rs	95.0%	1 Missing ⚠️
... and 3 more

Additional details and impacted files

@@          Coverage Diff          @@
##            main    #974   +/-   ##
=====================================
  Coverage   91.9%   91.9%           
=====================================
  Files         61      61           
  Lines      15964   15905   -59     
=====================================
- Hits       14673   14632   -41     
+ Misses      1291    1273   -18     

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

[joshka]

This looks good to me. I agree with all the changes generally (with a couple of questions / comments). I ended up just looking at the entire PR as I saw your comment about looking at each commit after I reviewed.

I like your thoughts on merging this as multiple commits generally, but I think I'm also fine with squashing the changes as this fits better with the workflow (and avoids having to split this into multiple PRs). There's only a few outstanding PRs right now, and the benefit of this PR outweighs the extra work to rebase them if needed.

I recall that there's a way to mark specific commits not to be included when annotating the file with git blame - perhaps we need that for this PR?

P.s. thanks for all the effort on this one - much appreciated!

[EdJoPaTo]

I applied the suggestions and added more lints. So its probably a good idea to review this completely again.

[joshka]

LGTM (I re-reviewed all the changed files since last review line by line).

I'd like to undo the changes to the u32 literals in the Color files, because the 3 byte values are actually more readable than the 4 byte values Perhaps keep the warn in the lint config, but add allows on the 3 files in question. d76a789 is the commit in question. Does that make sense?

I plan to merge this as a rebase - do you think it would be worth adding the PR number to all the commit messages to make it easy to link back to this PR and the comments (for anyone spelunking the codebase at some later date)?

Perhaps as a last thing to add for this, would you mind creating a .git-blame-ignore-revs file that marks these commits from blame (to make it easier to understand the blame output)? See https://git-scm.com/docs/git-blame / https://www.michaelheap.com/git-ignore-rev/ (or other blog posts for details).

[joshka]

P.s. this was really done well. I wonder if you might might consider writing an article / blog post on the approach you took to doing this in a controlled manner. Some things I'd really like to read about (and I'm sure others might feel the same) were:

• how - i.e. were you using an automated tool, iterating on cargo clippy --fix, using regex magic
• what / why? How were you picking the lints (just going down the pedantic list?) Which lints did you ignore based on experience and why?
• What problems did you run into while doing this?
• How long did it take
• How should the project move on from this - i.e. what's now enabled for all users.
• How easy was the process generally?
• What could make it better?
  etc.