-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Description
We currently don't have an RFC system, this is an experiment of starting a more RFC-like process before a larger change. I am also fairly busy right now so I am also creating this issue in the hope that somebody else may pick this issue up
Problem Statement
Currently, configuration in helix is closely coupled to serde/toml. Specifically, configuration is handled by derive(Desirilaize) on a struct. This approach was easy to get started with but doesn't scale well:
- Every config option requires a handwritten struct field.
- If we want to overwrite a config option in the language config we need to add that config to the language config aswell. This doesn't scale, its impractical to duplicate the entire config
- This won't work for additional configuration added by plugins
- Similarly if we want to overwrite a config option per buffer that requires yet another handwritten field on
Document
(which already polluted by too many fields). - The current config is not well suited for mutation. The
:set
and:toggle
commands essentially serialize the current config to json, edit them and desiralize again. Serde however (especially with our many custom deserilaizer) doesn't necessarily roundtrip correctly through json.- This becomes especially relevant with scheme plugins/scripts where the config should be both easily (and efficiently) readable and writable
- Overwrite/Inheritance/merging is inconsistent (we have an open issue about that) with regard to depth
- inspecting config options (and including documentation like we do for commands) and auto-generating docs is not possible/very hard
Prior work
The solution I would like to see is something that mirrors kakoune. I quite liked how simple yet flexible the config is. Especially the concept of scopes works quite well IMO. However, our needs are slightly more complex since kakoune doesn't have language config/LSP (which I would like to stay effortless). I cameup with a way to also use the scope system for these use-cases.
Proposed Solution
-
completely separate our config model from serde
- we plan to eventually move to scheme and abandon the toml config.
- until then we will have to implement (a backwards compatible) compatibility layer.
-
All options will have a simple canonical in-memory representation that is easy to read and modify. Convenience syntax should be handled during parsing.
- example: instead of
auto-pairs
accepting either a map of autopairs (like{ "(" = ")", "{" = "}"
) or a bool to indicate whether they are enabled simply add two separate options:auto-pairs.enable
andauto-pairs.pairs
. This allows consistent merging/inheritance/defaulting and doesn't require weird special cases in the config system.
- example: instead of
-
config options are represented as a flat called
OptionManager
.- Example:
softwrap.enable
andsoftwrap.max-wrap
are two entirely separate config options. The only thing that links them is a naming convention. - Grouping can be handled by scheme macros/serde compatibility layer
- This makes merging trivial (simply iterate all
OptionManager
s and stop at the first one that contains the required config option (some cases may also separate from value merging, this is separate, more on that later)
- Example:
-
Each scope where you may wish to set a config option has an
OptionManager
with the following priority: `split (do we want that?) -> buffer -> language -> global- This allows for very flexible
:set
(and similar) commands::set buffer line-ending crlf
,:set lang:rust softwrap.enable true
- This allows for very flexible
-
All of these options managers have identical capabilities, so any config option can be set anywhere (what was plaintext language config in the past is now simply in the global config)
-
There is a single
OptionRegistry
that stores metadata like documentation and validators (and ofcoruse which options even exist) -
Language servers are the only configuration that does not work in this system. We could make them dict values but that feels second class to me. Instead, I propose to simply create a special kind of scope for language servers with its own
OptionRegistry
that would be manipulated with:set lsp:rust-analyzer command rust-analyzer nightly
. LSP options would be essentially an opaque value field (so still no merging there but I still think that is correct) -
To address some config options that we may still want to merge I again took inspiration from kakoune: Kakoune has :
--append
and--remove
flags for:set
. This allows merging collections (I think we should maybe call them --mege and --subtract instead and also allow specifying a depth):set --mege=2 lsp:rust-analyzer options { "check" = { "command" = "clippy" }}
.- Something similar should be made available in the declarative scheme config.
Roughly the documentation I came up would look as follows:
pub enum Value {
List(Box<[Value]>),
Dict(Box<IndexMap<String, Value, ahash::RandomState>>),
Int(usize),
Bool(bool),
String(Box<str>),
}
impl Value{
fn from_str(&str) -> Self { .. }
fn to_str(&self) -> String { .. }
}
pub struct OptionInfo {
name: String
description: String,
validate: fn(&Value) -> anyhow::Result<()>,
completer: CompletionFn
}
pub struct OptionRegistry {
options: HashMap<String, OptionInfo>,
}
impl OptionRegistry {
fn register(&self, name: String, opt: Option) -> bool {
if self.options.contains(&name) {
return false
}else{
self.options.insert(name, opt)
}
}
fn get(&self, name: &str) -> Option<&Option>
}
pub struct OptionManager {
vals: HashMap<String, Value>,
}
impl OptionManager {
pub fn get(&self, option: &str) -> Option<Value>{
self.vals.get(option)
}
pub fn set_unchecked(&mut self, option: &str, val: Value) -> Option<Value>{
self.vals.insert(option, val)
}
pub fn set(&mut self, option: &str, val: Value, registry: &OptionRegistry) -> anyhow::Result<Option<Value>>{
let Some(opt) = self.registry.get(name) else { bail!("unknown option {option:?}") };
opt.validate(&val)?;
Ok(self.set_unchecked(option, val))
}
}
Unresolved Questions
- what should the syntax for
:set
and friends look like (from_str
andto_str
above)- In the exmaple above I used json syntax. I don't think that is ideal, as configuration will be done in scheme in the future and should look somewhat similar to not be surprising
- Maybe its better to just handle this with a scheme
:eval
to avoid introducing a duplicate syntax and remove the commands. - the latter would feel clunky (these commands are quite useful) so perhaps a compromise could be to simply evaluate the last argument (the value) as a scheme value?