config.SetGlobalOption no longer allows writing to settings.json

[nimishjha]

Description

In #3618, the ability for plugins to save settings was removed entirely. Surely this should be configurable, as some plugins depend on this capability?

The relevant code from action/command.go is below. Why not add a third argument writeToFile that gets passed through to SetGlobalOption instead of hard-coding false? I'd be happy to submit a PR.

func SetGlobalOptionPlug(option, value string) error {
        return SetGlobalOption(option, value, false)
}

Environment

• Version:
• OS:
• Terminal:

[dmaluka]

Surely this should be configurable

No, the very idea was to unconditionally guarantee that plugins won't mess with settings.json behind the user's back.

as some plugins depend on this capability?

Which plugins?

[nimishjha]

This plugin, and this one, for instance.

No, the very idea was to unconditionally guarantee that plugins won't mess with settings.json behind the user's back.

Surely this choice should be left up to the user? Seems a bit high-handed to just force-disable an existing feature to cater to one set of users, with no consideration for others. Are users children who can't be trusted to make their own choices?

[Andriamanitra]

This plugin, and this one, for instance.

No, the very idea was to unconditionally guarantee that plugins won't mess with settings.json behind the user's back.

Surely this choice should be left up to the user? Seems a bit high-handed to just force-disable an existing feature to cater to one set of users, with no consideration for others. Are users children who can't be trusted to make their own choices?

You can still work around it, although I would strongly encourage you to do that in your init.lua rather than in the distributed plugins because we don't want to go back to a situation where half of the plugin ecosystem silently rewrites settings.json/bindings.json files. Re-formatting them sorts the keys and removes all comments left by users which is super annoying.

[nimishjha]

I agree that micro's reckless reformatting of settings.json is a problem, I've been annoyed by it a few times myself. But there are correct solutions, and incorrect solutions. The correct solutions could have included:

• Fix the way micro saves its settings files so that it doesn't wreak havoc on the formatting, instead it searches for the relevant key-value pair and updates the value in-place.
• Have two settings files, one user-editable and the other micro-generated. Allow the user to choose which one takes precedence.
• Modify the existing behavior so that by default SetGlobalOption doesn't write to file, but give it an extra parameter that does enable writing to file.
• Create a new command SetGlobalOptionAndSave (the cheapest, quickest solution).

Instead, it was decided that an existing behavior that people have surely come to depend on would be silently changed. Consider a user who likes to toggle settings like the color scheme, word wrap, case sensitivity, etc. using keyboard shortcuts. He looks up the documentation, and sees SetGlobalOption. He proceeds to write plugins that use it, and that work fine for years. Suddenly one day he finds that they no longer work as expected. Micro doesn't show a warning along the lines of Plugin tried to write to file, which is now deprecated behaviour. He wastes time checking permissions, reinstalling micro, updating his OS, and reading the documentation which still does not say that Note: SetGlobalOption no longer writes settings to file. Finally he looks at the source and realizes the problem.

Is this how a quality piece of software should behave? I'm not asking for a massive rewrite. I'm saying that we can fix this obviously bad behavior in about five lines of code. It would be the right thing to do.

[Andriamanitra]

Having to break existing behavior was unfortunate but now that it's done I don't think we should re-introduce the problematic behavior.

• Fix the way micro saves its settings files so that it doesn't wreak havoc on the formatting, instead it searches for the relevant key-value pair and updates the value in-place.

While this would be a nifty feature it's a tricky one to implement correctly, and I'm not convinced humans and machines editing the same file is a good enough idea to warrant the increased complexity.

• Have two settings files, one user-editable and the other micro-generated. Allow the user to choose which one takes precedence.

I think better way to do this is for the plugins that need it to manage their own configuration files to avoid conflicts with other plugins & configuration. It can then use SetGlobalOption on init to set the value without ever touching settings.json.

• Create a new command SetGlobalOptionAndSave (the cheapest, quickest solution).

I recommend considering the other options first but if you really want to do this you could simulate running the set command which (unfortunately) still overwrites settings.json, eg. micro.CurPane():HandleCommand("set abc xyz").

Instead, it was decided that an existing behavior that people have surely come to depend on would be silently changed. Consider a user who likes to toggle settings like the color scheme, word wrap, case sensitivity, etc. using keyboard shortcuts. He looks up the documentation, and sees SetGlobalOption. He proceeds to write plugins that use it, and that work fine for years. Suddenly one day he finds that they no longer work as expected. Micro doesn't show a warning along the lines of Plugin tried to write to file, which is now deprecated behaviour. He wastes time checking permissions, reinstalling micro, updating his OS, and reading the documentation which still does not say that Note: SetGlobalOption no longer writes settings to file. Finally he looks at the source and realizes the problem.

We don't have a system to show deprecation warnings, the best way we have to communicate about changes is the release notes but it's understandable that not every plugin author/user reads them before upgrading. To be honest the whole plugin system is a bit of a hack, it exposes so much of the internals that we can't guarantee stability between versions. A note about the change in the documentation would be useful though (PRs welcome!).

[nimishjha]

I recommend considering the other options first but if you really want to do this you could simulate running the set command which (unfortunately) still overwrites settings.json, eg. micro.CurPane():HandleCommand("set abc xyz").

And how long until that capability is also removed?

We don't have a system to show deprecation warnings

An infobar message, or a log message would have done the job.

What I'm hearing is "It's a hack, and we're ok with that." Disappointing, but oh well.

[dmaluka]

Personally I was assuming that if some plugins are using SetGlobalOption*(), their intention is to set the option globally for all buffers (as opposed to setting it for the given buffer only, via SetOption()), and the fact that the setting is also saved to settings.json is not the intention but only a side effect of micro's poor design decision to mix together these two different things (setting globally vs per-buffer and setting permanently vs for current micro session).

To be honest the whole plugin system is a bit of a hack, it exposes so much of the internals that we can't guarantee stability between versions.

Yeah, very true. That being said, those APIs that are explicitly documented in plugins.md are a different story: we do try to keep them stable.

So in this case, it is our bad: we changed the documented behavior of SetGlobalOption*() and TryBindKey(), with good intentions (to mitigate a harmful consequence of micro's poor design decisions), without realizing that some plugins might intentionally rely on this behavior (and that some users might actually want those plugins to do that).

[JoeKar]

Looks like we need an additional option to allow overwrites, since the user shall have the control about overwriting its own settings.json or bindings.json and not the plugins.
Or do we have better ideas to solve all the (known) needs?

[nimishjha]

I think that you folks' view of the problem is shaped by a long history of angry complaints from users that micro clobbered their settings files. The association of SetGlobalOption with user complaints has understandably caused you to view it negatively and I think that's coloring your decisions.

It's never a poor design decision to have a powerful plugin system (or a powerful tool, or a powerful programming language). Imagine how much worse the history of computing would be if languages like C didn't have powerful features like manual memory management, just because some people can't do it well. Making a plugin system less powerful because some people complain is like mandating the removal of the sharp tip from knives because some people use them to attack other people, or limiting the top speed of cars to 20 mph because some people are terrible drivers.

The actual problem here is that the collection of approved plugins already uses SetGlobalOptions and that, combined with micro's way of reformatting settings, has caused problems. I'll say again: the problem is not that micro's plugin system is powerful. The problem is that its power, combined with the reformatting behavior, combined with the fact that official plugins cause that behavior can be annoying to some users.

To me, the solution seems obvious. The change to SetGlobalOption can stay because it solves the problem of continual complaints from users. It's trivial to create a new action SetGlobalOptionAndSave. The advantage to this is that since official plugins go through an approval process, it's easy to check for this new action and block a plugin from the official channel until it's removed. And those people who do want to control their settings through plugins can do so. Everybody wins.

[Andriamanitra]

What's wrong with the suggestion to use separate config file?

• Have two settings files, one user-editable and the other micro-generated. Allow the user to choose which one takes precedence.

I think better way to do this is for the plugins that need it to manage their own configuration files to avoid conflicts with other plugins & configuration. It can then use SetGlobalOption on init to set the value without ever touching settings.json.

This approach allows plugins to persist configuration without modifying settings.json. The best part is that it doesn't rely on any new API functions so it doesn't limit the plugin's compatibility to >=2.0.16. The only downside I can think of is that it's a couple more lines of code for the plugin to implement, but in my opinion it's well worth it.

[nimishjha]

The problems with your suggestion:

• It's inelegant. Micro starts up, loads its config, then the plugin loads its config, and applies the changes. Is there a flicker as the colorscheme changes? That would be annoying.
• It's misguided. I actually want to manage my settings through plugins, while you are maintaining as an article of faith that plugins must not touch settings.json.
• It would actually create more confusion among users. If settings.json has one color scheme, and case-sensitivity set to off, and word-wrap set to off, and a plugin has different settings, it would be very confusing to the user to check the settings file and see that everything is as expected, but within micro all the actual settings are different. What happens if multiple plugins have conflicting settings? Does the order of loading of plugins make a difference? Settings should be in one place.

My suggestion to have two config files is different: Micro would load both files at launch, and generate a final config derived from both files, and use that. So even though you have two config files, they're effectively one config.

The more I think about it, though, the more I think we should fix the actual problem: stop Micro from clobbering the settings files. It's trivial to parse the file line by line, find the relevant setting, and change its value without affecting the rest of the file. This is how it Should Have Been Done in the first place. I'd be happy to do it.

This would solve the actual problem, instead of trying to come up with ugly workarounds. Users are likely to use set..., which will clobber their settings file anyway.

[Andriamanitra]

• It's inelegant. Micro starts up, loads its config, then the plugin loads its config, and applies the changes. Is there a flicker as the colorscheme changes? That would be annoying.

I don't think it's inelegant. There's no flicker. Try it out: https://gist.github.com/Andriamanitra/36371cb2b22bb41889ac62f6f596aa18

• It's misguided. I actually want to manage my settings through plugins, while you are maintaining as an article of faith that plugins must not touch settings.json.

You would still be managing settings through plugins, just not through settings.json. The only practical difference is the filename.

• It would actually create more confusion among users. If settings.json has one color scheme, and case-sensitivity set to off, and word-wrap set to off, and a plugin has different settings, it would be very confusing to the user to check the settings file and see that everything is as expected, but within micro all the actual settings are different. What happens if multiple plugins have conflicting settings? Does the order of loading of plugins make a difference? Settings should be in one place.

This is a valid concern but the plugin's documentation would hopefully be clear about it. Most plugins shouldn't need the feature.

My suggestion to have two config files is different: Micro would load both files at launch, and generate a final config derived from both files, and use that. So even though you have two config files, they're effectively one config.

That's exactly what the plugin would do, too. The only difference is that it would be implemented in Lua as a plugin instead of in micro itself.

The more I think about it, though, the more I think we should fix the actual problem: stop Micro from clobbering the settings files. It's trivial to parse the file line by line, find the relevant setting, and change its value without affecting the rest of the file. This is how it Should Have Been Done in the first place. I'd be happy to do it.

This would solve the actual problem, instead of trying to come up with ugly workarounds. Users are likely to use set..., which will clobber their settings file anyway.

I would appreciate this feature, but doing it line-by-line won't work because there may be multiple keys on a single line or multi-line strings or nested keys or other weird formatting – you need an actual parser.