search

ebkalderon / tower-lsp

Language Server Protocol implementation written in Rust
Apache License 2.0
962 stars 54 forks source link

Nightly rustfmt #315

Closed silvanshade closed 2 years ago

silvanshade commented 2 years ago

I noticed in 84d7bfc2275e69e5faf497a731e2379e28a3ee15 that you changed the cargo fmt job to the stable toolchain. I think this is fine but I guess I should explain why I would using +nightly to begin with. In lspower I had a custom rustfmt.toml which uses several nightly features (which for some reason have existed for a long time but not been moved to stable).

Anyway, I enabled those settings here and created a PR to compare. I don't think it's really important to merge unless you prefer this formatting as well, but at least I think the merged imports are quite nice. (I'd be happy to apply the formatting to those manually as well).

ebkalderon commented 2 years ago

Thanks so much for the pull request, @silvanshade! :heart: I can definitely see now why +nightly was used originally.

While I'm not entirely against the formatting changes, and I do really like the look of the increased max line width with some of the longer function and method signatures, I do intentionally strive to stick to standard rustfmt settings as closely as possible and whenever possible. I presume the majority of downstream users will likely be working with some version of the standard configuration, and I would like to do the same to motivate and inform the design of the public tower-lsp API, i.e. to avoid excessive line breaks in most common usage.

Still, I may consider adding this again down the road as the standard rustfmt configuration changes. For this reason, I think this pull request is still quite welcome and valuable. Thank you again for bringing this to my attention!