Continue loading the config on error

[antoyo]

Fix #1732

There are cases where this improve the error that is shown to the user, but in some cases, the error message gets worse, i.e. it loses its context (key name) and/or position.
I'll try to find a way to improve this, but I'm not sure how, so suggestions are welcomed! (I tried serde-path-to-error, but it does not seem like it works when called from ok_or_default.)

I'm submitting this PR now to gather early reviews.

[pickfire]

I think doing global variable for this will make stuff hard later, doing it as variable passing in rust is more common and less error prone.

[archseer]

Hmm, rather than partial parsing, how about reloading the config via #1803 but if the config fails to parse we don't swap to the new config?

That won't help if you start the editor with a broken config, but at least it won't break your setup if you're working on your config and "live reloading".

[antoyo]

Hmm, rather than partial parsing, how about reloading the config via #1803 but if the config fails to parse we don't swap to the new config?

That won't help if you start the editor with a broken config, but at least it won't break your setup if you're working on your config and "live reloading".

I see this as a different feature.
As a user of an alternative keyboard layout, I find it very inconvenient if I ever have to edit a file without my remapped "hjkl", so I find this PR very important.

Is there anything you don't like about it?

[antoyo]

@archseer Any update on this?

[pickfire]

I think if we do it this way with global static variable, later for reloading config, we need to handle this, I wonder if it would be better if we can just propagate the error upwards.

[antoyo]

The problem is that serde is it's own thing, so it's not that easy.
Anyway, I'm still waiting for the answer in the discussion as it seems some people are against the idea of this PR.

Maybe the way this is handled will change after we discuss the design.

[antoyo]

@archseer @pascalkuthe @the-mikedavis @sudormrfbin: I'd like to get some thoughts on this PR. Is it that you don't want to support this in Helix? If so, I could also do a simpler version where it only loads key mappings in case of a bad config. If that's really unwanted, any alternative you would consider for the use case of having a remapped hjkl when a config file is broken?

[the-mikedavis]

IMO it's not worth adding to or complicating the config parsing code to cover this case. It seems to me that having a broken config file should be a rare and temporary problem, especially with how error messages have improved (TOML parsing failures with the toml crate update in #5656 and otherwise invalid commands/configs in #3931). Specifically about hjkl and motion: the default config should have mouse support and arrow keys enabled so even if it isn't very comfortable, you should be able to move around in your config file to fix issues.

If you're trying out different configs often you could work around errors by using the -c/--config CLI flag either to point to the experimental config or a safe minimal one you know will work. Or you could use :config-reload when editing the config file which will not apply the config if there are errors.

[antoyo]

Ok, I understand.

Would you also be against something that would attempt loading multiple config files or a fallback config file?

[pascalkuthe]

Ok, I understand.

Would you also be against something that would attempt loading multiple config files or a fallback config file?

The fallback config for helix is the default config. Helix prides itself on having a good default config so its a pretty decent fallback. Adding cascading fallbacks for the global config.toml seems a little odd to me since it's unclear in what order they would be loaded and you can still mess up those files as well so it wouldn't really fix the issue. As Mike mentioned you can easily use arrow keys on international keyboards that don't have hjkl at all. Helix config files are intended to be very simple so if you run into problems just comment out the offending lines with c-c and :config-reload. You can also just start helix with a working config and edit the config while testing with :config-reload while editing the config file until the new config works.

The markdown config is a stopgap until a scripting language (scheme) based config is implemented in the future. I think we want to keep the config-related code as simple as possible until then and with a script partial config evaluation becomes basically impossible anyway.

[antoyo]

Ok, thanks for the explanation.