Skip to content
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

Read rust-analyzer configuration from Cargo.toml? #6113

Closed
cuviper opened this issue Oct 1, 2020 · 15 comments
Closed

Read rust-analyzer configuration from Cargo.toml? #6113

cuviper opened this issue Oct 1, 2020 · 15 comments
Labels
A-config configuration S-actionable Someone could pick this issue up and work on it right now

Comments

@cuviper
Copy link
Member

cuviper commented Oct 1, 2020

Would it be possible to read configuration options from Cargo.toml? Then they could be specified in an editor-neutral way, something like:

[package.metadata.rust-analyzer.cargo]
noDefaultFeatures = true
features = ["foo", "bar"]

I think snake-case would be more idiomatic for Cargo.toml, but I don't know how deep we'd want to get into translating names. There's also a mismatch between the usual default_features = false and RA's noDefaultFeatures.

@lnicola
Copy link
Member

lnicola commented Oct 1, 2020

Depending on the priorities, a possible downside here (for open-source projects) is that you might get the settings the project author likes, not what you'd like to use.

@cuviper
Copy link
Member Author

cuviper commented Oct 1, 2020

True, and I would suggest it be used sparingly for that reason.

I was thinking along the lines of [package.metadata.docs.rs], which is relatively limited: https://docs.rs/about/metadata

@kjeremy
Copy link
Contributor

kjeremy commented Oct 2, 2020

Depending on the priorities, a possible downside here (for open-source projects) is that you might get the settings the project author likes, not what you'd like to use.

Could user specific overrides go into .cargo/config?

@lnicola
Copy link
Member

lnicola commented Oct 2, 2020

Could user specific overrides go into .cargo/config?

Traditionally, cargo/.config is completely different from Cargo.toml.

@jonas-schievink
Copy link
Contributor

.cargo/config sometimes gets checked into repos too

@matklad matklad added the S-unactionable Issue requires feedback, design decisions or is blocked on other work label Oct 14, 2020
@matklad
Copy link
Member

matklad commented Oct 14, 2020

Tough question.

On the one hand, this is definitely an upstream issue. Configuration handling in LSP is just a mess, and "ability to share lsp config in an editor-agnostic way" is something that, imo, should be solved at the protocol level.

On the other hand, I doubt that this will ever be fixed upstream.

So we can do one of the following:

  • punt on upstream and do nothing
  • beat purity with practicality and support rust-analyzer.toml
  • rebel against upstream, introduce lsp_config.toml and advertise it to authors of other LSP servers as well.

@flodiebold
Copy link
Member

Configuration handling in LSP is just a mess, and "ability to share lsp config in an editor-agnostic way" is something that, imo, should be solved at the protocol level.

I feel like consistency with other Rust tooling would be more useful than consistency with other LSP servers anyway?

@therealprof
Copy link

I was just pointed to this issue since we've been having the discussion on the rust-embedded channel. I fully agree with the motivation and would like to second it.

Regarding the actual implementation, I'd very much favour:

  • beat purity with practicality and support rust-analyzer.toml

That'd allow projects to check in a useful configuration (those actually do exist for embedded cases) and in other cases people to create their own preferred configuration without having to work around Cargo.toml modifications all the time.

@matklad
Copy link
Member

matklad commented Dec 9, 2020

Somewhat related, #6776 documents our current configs.

@flodiebold
Copy link
Member

IMO we should go with rust-analyzer.toml (or maybe .rust-analyzer.toml).

@flodiebold flodiebold added S-actionable Someone could pick this issue up and work on it right now and removed S-unactionable Issue requires feedback, design decisions or is blocked on other work labels Jan 20, 2021
@lnicola
Copy link
Member

lnicola commented Jan 20, 2021

And maybe a separate setting that enables or disables support for it (to give users the final say, unless the project author overrides it in .vscode/settings.json 😕).

@matklad
Copy link
Member

matklad commented Jan 20, 2021

I do think that editor's preferences should override the .toml. Implementation wise, we'd need some way to prevent VS Code from stomping over the config though -- it sends client-side defaults even if user's config is empty :(

@lilyball
Copy link

I really want something like this so I can enable features during development that aren't part of the default feature set. I work in a large workspace so I can't just turn them on in VSCode's Workspace settings as the features only apply to a specific crate within the workspace. So being able to read them from Cargo.toml or from a rust-analyzer.toml file would be extremely helpful.

And on that note, any such support for this does need to handle the case of a project within a workspace (in my case, a virtual manifest workspace). Even if I have the whole workspace open in VSCode, rust-analyzer should pick up feature settings on a per-crate basis for each workspace member.

@lnicola
Copy link
Member

lnicola commented Nov 2, 2022

@Veykril should we close this in favor of #13529?

@Veykril
Copy link
Member

Veykril commented Nov 2, 2022

Yes, I think everything in here has been noted down in that issue (except for one small thing that I'll add).

@Veykril Veykril closed this as completed Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-config configuration S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

9 participants