feat: load local config

[AlexanderBrevig]

This will enable having a .helix/config.toml in a project repo and have it override default and ~/.config/helix/config.toml settings.

This behaviour is controlled with config.toml:

[editor.security]
load-local-config = false
confirm-local-config = true

The default keeps Helix working as it is. If you only set load-local-config = true then you'll be promted to answer yes before loading. This prompt can be removed by setting confirm-local-config = false.

[AlexanderBrevig]

Thanks for the review and input @kirawi! I've fixed the requested changes (an embarrassing reminder that late night sessions may lead to weird things...) and fixed clippy lint errors locally.

[diktomat]

This should be optional, as one may not want to use custom config coming with someone else's repo and, more important, this could harm the user's computer, for example by resetting a common key bind to some harmful shell command.

[the-mikedavis]

That probably falls under something like https://github.com/helix-editor/helix/issues/2697 (or is similar in spirit). It's notable that we already merge in .helix/languages.toml and I think you could easily craft a malicious languages.toml like so:

[[language]]
name = "toml"
language-server = { command = "rm", args = ["-rf", "/"] }

[AlexanderBrevig]

I've added this to config.toml

[editor.security]
load-local-config = false
confirm-local-config = true

(commented out, you will also see -languages versions for a future PR)

That's the default. If you toggle both, then Helix will load $PWD/.helix/config.toml without asking to do so.

What do you think? I very much like the feature, but I am not married to the implementation details. I'll fix anything you see fit.

[AlexanderBrevig]

Updated the docs and merged with master. hx -c ~/team/hx/config.toml will now load the "team" config, and then ask to load $PWD/.helix/config.toml if [editor.security] load-local-config = true.

Anything I can do to help this PR forward? (I understand the team is under a lot of load)

[AlexanderBrevig]

I've fixed your suggestions @kirawi, do you have some more feedback or questions? :)

[AlexanderBrevig]

NOTE: I merged in master to try and fix a bug that's been introduced somehow. Details below.

Huge thank you @pickfire I can see that I maybe wrote this code a bit prematurely in terms of how well (let's be honest, bad) I knew Rust.

However, the strange thing now (before the refactor too) is that I seem to lose ability to input text after the editor is loaded. It did not use to be that way.

Reproduce: Add [editor.security] load-local-config = true and maybe some config file in ./helix/config.toml. Then cargo run . will first ask you to type yes<RET> if you have a file, then open the file picker - here you can type. When you open a file, suddenly I cannot type any more. I have very little idea where to check, and somehow doubt that its this code that introduces this error. ~~Unless std::io::stdin().read_line somehow blocks the events coming in after...?~~

I did some cleanup in addition to yours. It is better now, at least. A bit unsure if the functions should be in config.rs or simply at the bottom of main.rs.

[AlexanderBrevig]

load_local_config part seemed okay to me but I am not sure about the confirm_local_config part.

I am in favor of removing the confirmation prompt. Should I do it?

[pickfire]

I am in favor of removing the confirmation prompt. Should I do it?

Yes, I think for now we can keep it with a small change to only have a config to allow loading local config. That would be easier review and try it out.

[the-mikedavis]

The security aspect of this still bothers me a bit - it ends up really nerfing the feature. I think it's silly in the same way that git's cve-2022-24765 (the safe.directory one) is silly, and to my knowledge no other terminal based editor is careful about this. I would argue at least that this should be enabled by default.

[AlexanderBrevig]

The security aspect of this still bothers me a bit - it ends up really nerfing the feature. I think it's silly in the same way that git's cve-2022-24765 (the safe.directory one) is silly, and to my knowledge no other terminal based editor is careful about this. I would argue at least that this should be enabled by default.

Since three of four voices in this PR seems to agree, I'll change the default.

[AlexanderBrevig]

Somehow I totally forgot this. Sorry.

[Nazariglez]

Hello fellows! This feature looks great. What needs to be done to be able to merge this? Thanks

[gibbz00]

Hello fellows! This feature looks great. What needs to be done to be able to merge this? Thanks

I did a refactor on config loading, https://github.com/helix-editor/helix/pull/5636. ~~I'm in the process of merging~~ Merged this PR with that branch, hoping that it can help getting this one merged too.

[Nazariglez]

That's awesome @gibbz00, thank you!

[poliorcetics]

I Hope I'm not too late, but I would like helix to also support (config|languages).local.toml, that way I can easily have a global gitignore for it and avoid polluting repos with my configs and/or ignores.

The exact behavior can either be local replaces the non-local file or is merged with it, favoring the local one (I prefer the second option but both can be argued for)

[pascalkuthe]

I Hope I'm not too late, but I would like helix to also support (config|languages).local.toml, that way I can easily have a global gitignore for it and avoid polluting repos with my configs and/or ignores.

The exact behavior can either be local replaces the non-local file or is merged with it, favoring the local one (I prefer the second option but both can be argued for)

What exactly is the advantage of config.local.toml over .helix/config.toml in that regard? I think adding .helix/** to the global gitignore would be enough (that would also take care of .helix/languages.toml)

[poliorcetics]

As helix becomes more used, I expect some repos will start having a config in their git, like many already do for VSCode, jetbrains IDEs or vim/neovim, in this case .helix/** won't be enough

[the-mikedavis]

.helix/languages.toml and .helix/config.toml also work in the global gitignore. I also don't see the advantage to using config.local.toml and languages.local.toml: it should be equivalent to .helix/config.toml and .helix/languages.toml respectively, no?

[poliorcetics]

No, what I mean is, in the project repo, people will start committing <my repo>/.helix/{config,languages}.toml (and ideally it should be .config/helix/... even locally, like cargo-nextest does), and having the hability to override it locally without having to play with the Git index would be nice.

As an example, search path:/.vscode/settings.json. The results are murkier for Vim and Jetbrains IDEs but there are examples.

[the-mikedavis]

Oh I see, config.local.toml would be in addition to .helix/config.toml, right?

For scenarios like this I think it would be better to make use of the -c <config-file> CLI option and have a wrapper around hx -c to control which config is used based on the directory it's called in

[poliorcetics]

For scenarios like this I think it would be better to make use of the -c <config-file> CLI option and have a wrapper around hx -c to control which config is used based on the directory it's called in

That seems like a very fast path to the disaster that is using $CARGO_TARGET_DIR globally and doesn't allow for merging configs in the local -> project -> user order, because projects may be settings configs to help users go in more smoothly and having to pass -c is simply a workaround

[charles-dyfis-net]

The security aspect of this still bothers me a bit - it ends up really nerfing the feature. I think it's silly in the same way that git's cve-2022-24765 (the safe.directory one) is silly, and to my knowledge no other terminal based editor is careful about this. I would argue at least that this should be enabled by default.

Both vim and emacs disallow modelines from configuring settings that can result in executables being run by default / without user approval.