AstroNvim / astrocommunity

A community repository of common plugin specifications
GNU General Public License v3.0
1.2k stars 242 forks source link

Remove opinionated rust-analyzer config from `astrocommunity.pack.rust` #1109

Closed boraarslan closed 3 months ago

boraarslan commented 3 months ago

Is your feature related to a problem?

https://github.com/AstroNvim/astrocommunity/blob/b882cbb55f5c837736f6db5d0e2c3ac7677f3fcb/lua/astrocommunity/pack/rust/init.lua#L19-L47

There are multiple problems with this config.

https://github.com/AstroNvim/astrocommunity/blob/b882cbb55f5c837736f6db5d0e2c3ac7677f3fcb/lua/astrocommunity/pack/rust/init.lua#L29-L32

This is entirely wrong and useless. These options are not under "assist". The correct namespaces are rust-analyzer.imports.granularity.enforce and rust-analyzer.imports.granularity.group

https://github.com/AstroNvim/astrocommunity/blob/b882cbb55f5c837736f6db5d0e2c3ac7677f3fcb/lua/astrocommunity/pack/rust/init.lua#L38-L43

This just makes the inlay hints way too verbose. Most of the time, the lifetime elision hints are not helpful at all and just add noise.

https://github.com/AstroNvim/astrocommunity/blob/b882cbb55f5c837736f6db5d0e2c3ac7677f3fcb/lua/astrocommunity/pack/rust/init.lua#L33-L37

This is just a bad default. I'm not sure why this is disabled.

https://github.com/AstroNvim/astrocommunity/blob/b882cbb55f5c837736f6db5d0e2c3ac7677f3fcb/lua/astrocommunity/pack/rust/init.lua#L23-L28

This is controversial, to say the least. Using clippy as the check command causes very long feedback loops on larger projects as it's slower and heftier than cargo check.

Describe the new feature

The default rust-analyzer configuration is adequate for most people. This opinionated configuration should be removed as it doesn't offer any benefits over the default one.

Additional context

No response

Uzaaft commented 3 months ago

Feel free to open a PR @boraarslan

Uzaaft commented 3 months ago

Most of what you point out makes sense. But I find myself disagreeing with the clippy bit. The pack is there to provide a default for new users of astronvim, who might not know how to configure an LSP. It should stay IMO

boraarslan commented 3 months ago

@Uzaaft I'm not really opposed to defaulting to clippy; if I were to suggest something, I would've suggested that README should point out the default check command is not being cargo check and what the user should do in case they want to switch to the default behavior. IMO, doing the exact opposite (defaulting to cargo check and adding the steps needed to switch to cargo clippy to the README file) is better, but as I said, I don't have a strong stance on that.

Uzaaft commented 3 months ago

Let's document the cargo clippy behaviour, I think that makes sense. Also, you're overestimating peoples ability to read the docs and make changes. 😂

boraarslan commented 3 months ago

Ahahahah, maybe you are right. I've updated the PR to include the documentation changes.

Uzaaft commented 3 months ago

Grazie Mille @boraarslan