-
Notifications
You must be signed in to change notification settings - Fork 1k
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
use rustfmt #1668
Comments
Yes please, it is a real drag to have to turn off format on save in your editor everytime one wants to work on rust-libp2p when ones daily work is on a rust code base that uses rustfmt. It also means I can't have a buffer open doing rust-libp2p work and a buffer open to other repos without have to reset all the formatting changes, while manageable its a drag to have to think about this stuff. Can we configure rustfmt in a manner that is acceptable to the rust-libp2p project? |
Rust was designed to be expressive, which makes Rust far harder to fmt than languages explicitly designed to be inexpressive, and so rustfmt ends up being fairly crappy. We could set rustfmt.toml to As luck would have it, we could exploit rustfmt's bugginess for this by simply doing https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=856d116c793b74e6809441b4ccf7af68 in lib.rs or somewhere so that rustfmt panics rust-lang/rustfmt#3678 |
@burdges out of curiosity: could you point out examples where the rustfmt formatted code is so bad? I guess the huge lists of type constraints on some parts of the code will look bad. Maybe these sections can be manually excluded. In general I think fmt does an OK job. |
I only know my own code, not the libp2p crate, but.. It screws up any whitespace that communicates emphasis, especially with iterator chains like https://github.com/w3f/schnorrkel/blob/master/src/vrf.rs#L944 so you need far more temporaries, which becomes excessive. There are many situations even post-NLL where Rust requires closures be handled in line, not as temporaries. Anyways Parity has their own style guide and afaik they ban rustfmt from their code bases. I hope this becomes more commonplace.. |
Actually there are parity codebases that use rustfmt. Gav doesn't like it so it's not used in substrate or polkadot, but many smaller crates/repos it's up to the devs basically. Also note that this is a libp2p repo not a paritytech repo. |
While my preference is for consistent formatting that doesn't require devs to learn a new style guide for each repo they contribute to, finding a way for rustfmt to simply ignore and not do anything is an acceptable solution for me. Note that code style is in many ways subjective and on a large project with many devs there is going to be inconsistent formatting even with a style guide. |
I think in 95% of the time rustfmt does an OK job, and for the remaining 5% I am fine with manually disabling it. But if that is not an option, just disabling it for the entire crate would already be an improvement for me. |
Any solution that doesn't require me to change my editor config to work on the repository is fine for me. Consistency is good, the granularity doesn't matter much I'd say i.e., more than one style in a repo but consistent within xyz modules - is libp2p really big enough or old enough to warrant that? |
Yes and pretty-please. It's going to make a pretty big diff the first time, which no one likes. But the longer we wait, the worse that diff is going to be. But it's still worth the initial diff noise. It's helpful for maintainers and reviewers in that they don't have to bother with asking formatting changes on PRs. It helps first time contributors (I'm feeling this right now) by not having to guess the projects style.
I feel you, and empathize. In the cases where it legitimately makes for illegible code, locally disabling is an option. But I haven't seen rustfmt do that since maybe the first few releases. |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Why use rustfmt? The main reason is so you don't annoy developers when they accidentally run it before committing their changes.
The text was updated successfully, but these errors were encountered: