Stream: t-compiler/wg-rls-2.0

Topic: Feature flags


std::Veetaha (Feb 17 2020 at 10:47, on Zulip):

@matklad , might I ask you why we shouldn't expose feature flags structure to UI, @Laurențiu Nicola and I are convinced that this affects UX... We would like to declare them in package.json at least temporarily, while we don't generate it from our source code

matklad (Feb 17 2020 at 10:49, on Zulip):

I don't know what's the right answer here

matklad (Feb 17 2020 at 10:50, on Zulip):

I would be 100% with generating that bit of settings from a Rust declaration

matklad (Feb 17 2020 at 10:50, on Zulip):

I would be less OK with just manually syncing things, or keeping source of truth in package.json

matklad (Feb 17 2020 at 10:50, on Zulip):

Opened https://github.com/rust-analyzer/rust-analyzer/issues/3189 for the overall configuration design discussion

Laurențiu Nicola (Feb 17 2020 at 11:11, on Zulip):

I think sync'ing them manually is fine for now because 1. this affects users right now, 2. there's very few of them, 3. if we use a prefix to differentiate these preferences, the client-side work to support it (notifying the server that a flag has changed) will be useful when switching to something generated automatically

matklad (Feb 17 2020 at 11:13, on Zulip):

Yeah, that's reasonable. They reason why I am not 100% is that doing that relives pressure from this issue for VS Code and, all other things being equal, I'd prefer not to single out this editor more than it already is

Laurențiu Nicola (Feb 17 2020 at 11:17, on Zulip):

Still, generating the package.json parts automatically will not help other editors, will it? Because each LSP client will have a different way to configure these.

matklad (Feb 17 2020 at 11:18, on Zulip):

It will, as it will also generate docs. But this is just a minor point

Laurențiu Nicola (Feb 17 2020 at 11:23, on Zulip):

Then a plan here might be to:

  1. manually add the settings to package.json and implement a way to toggle them from the Code extension
  2. replace the server hashmap with a strongly-typed struct (alternatively, use a different self-documenting format)
  3. extend xtask to read the doc comments on that struct (or the different format) and produce docs for them
  4. extend xtask to generate the package.json stuff

We could also describe them in e.g. JSON schema and optionally generate Rust code from that.

matklad (Feb 17 2020 at 11:25, on Zulip):

SGTM, though I am not sure about 2. The problem main problem with strong types is that we want to separate LSP settings from analyzer settings. String types allow us to have a runtime dep there

std::Veetaha (Feb 17 2020 at 11:35, on Zulip):

We may declare feature flags structs specific to non-lsp crates and map lsp-feature-flags to non-lsp ones by impl From. Making them a hashmap I would think that we would want to dynamically mutate our feature flags, though this doesn't happen in practice, hence simple mapping is more convenient I think

Laurențiu Nicola (Feb 17 2020 at 11:37, on Zulip):

Or store them in a struct on the server and use strings at the interface

Laurențiu Nicola (Feb 17 2020 at 11:39, on Zulip):

@std::Veetaha generating code from the JSON schema should work, right?

Laurențiu Nicola (Feb 17 2020 at 11:40, on Zulip):

Can it also store comments/descriptions?

matklad (Feb 17 2020 at 11:40, on Zulip):

I'd rather avoid JSON schema and use the appoach we use to gen ast

matklad (Feb 17 2020 at 11:41, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/blob/master/xtask/src/ast_src.rs

matklad (Feb 17 2020 at 11:41, on Zulip):

declare a rust struct with a macro, generate things from this struct

matklad (Feb 17 2020 at 11:41, on Zulip):

that avoids build-time dependency on parsing json/toml/ron whatever

std::Veetaha (Feb 17 2020 at 12:48, on Zulip):

Jason schema do stores metadata "title", "description" props, tho I am totally okay with the approach we have in ast gen. But, @matklad, does my proposal with separating the API and mapping between different structs seem reasonable?

matklad (Feb 17 2020 at 12:49, on Zulip):

It does, though I think hash-map vs typed API is a minor point here

matklad (Feb 17 2020 at 12:50, on Zulip):

Like, I do thing that either would work fine, and that both would be indistinguishable for users

std::Veetaha (Feb 17 2020 at 12:50, on Zulip):

Right, indistinguishable, that's just a server impl detail

std::Veetaha (Feb 17 2020 at 12:51, on Zulip):

Sorry, I might've missed the point

std::Veetaha (Feb 17 2020 at 13:05, on Zulip):

Anyway, what was the initial reason we switched from Ron? By parsing Ron you mean time consuming?

matklad (Feb 17 2020 at 13:06, on Zulip):

Mainly to improve compile times, and also to reduce overall complexity

std::Veetaha (Feb 19 2020 at 09:44, on Zulip):

So, do we work on configuration design first? I don't think that adding feature flags declaration to package.json would be any noticeable tech debt, amending package.json is the only thing it requires...

matklad (Feb 19 2020 at 10:21, on Zulip):

I am ok with r+ whatever changes to package.json/TS to make better short term.

For backend changes, I think I'll need to spend some time familiarizing myself with the relevant LSP APIs first, and that probably won't happen this week

std::Veetaha (Feb 19 2020 at 10:26, on Zulip):

Okay, I'll add relevant feature flags to package.json then.

Also, as a side not, I'd suggest using r+ word not so often, because this is very ad hoc.

Last update: May 29 2020 at 17:30UTC