search

BurntSushi / walkdir

Rust library for walking directories recursively.
The Unlicense
1.21k stars 106 forks source link

chore(docs): change single character pattern #199

Closed joshuachp closed 2 weeks ago

joshuachp commented 2 weeks ago

Change the single character string to a char in the examples and README to fix the "clippy::single-char-pattern" warning.

BurntSushi commented 2 weeks ago

I don't really agree with this lint. I think it has dubious upsides and potential perf downsides, although I'd be hard-pressed to write a real world benchmark where it matters.

joshuachp commented 2 weeks ago

I was curious so I checked this out on godbolt.org and I'm pretty confident they are the same. The only difference is that using a string generates the clippy warning, but in the end it's a minor thing anyway 😄.

FYI: in the closure the only change I could find is a lea for the string, while there is a move for the char. While with opt-level = 2 the asm is the same as the char.

https://godbolt.org/z/68Mf9eKnW

BurntSushi commented 2 weeks ago

Yes, I was careful in my wording.

There are a lot of Clippy lints I don't really agree with. And I generally don't like PRs that just exist to fix those lints. Basically, I don't judge revisions to code based on whether they satisfy a lint tool. I judge them based on whether they improve the code. And in the grand scheme of things, I don't think find('a') versus find("a") embodies a distinction with a meaningful difference. You can use either and it will be fine.

BurntSushi commented 2 weeks ago

To be clear, I would like to eventually use Clippy in my projects. I do like lint tools. But I just haven't had the time or inclination to set it up. Because "set it up" means going through the lints and picking which ones I like and don't like. It's a lot of work.

joshuachp commented 2 weeks ago

Completely agreed, sorry if it seemed I was pushing this. You nerd sniped me with your comment and had to check this out 😆

I opened the PR just because in a new project I copy pasted the example and got the warning, so I though other people probably would encounter it too. Anyway, thank you for your time.