zyedidia / micro

A modern and intuitive terminal-based text editor
https://micro-editor.github.io
MIT License
24.42k stars 1.16k forks source link

command: Prevent re-writing settings in case of local option #3178

Closed JoeKar closed 2 months ago

JoeKar commented 3 months ago

This is a follow-up of #3042.

dmaluka commented 3 months ago

LGTM. I'd also suggest a small refactoring of SetGlobalOptionNative() as a follow-up: deal with local at the beginning and be done with it, before dealing with global:

if local {
    MainTab().CurPane().Buf.SetOptionNative(option, nativeValue)
    return nil
}
dmaluka commented 3 months ago

Upon a fresh look, it's also kinda weird that we check (and "properly handle") a local option inside a function named SetGlobalOptionNative(), as a special case, rather than checking that in the caller function. Or are we checking that both here and in the caller function? (I'm afraid that's what we actually do, and also I'm afraid that we cannot simply remove this redundancy, as it would break something.)

But that's just a lyrical note.

JoeKar commented 2 months ago

But that's just a lyrical note.

What about renaming SetGlobalOption() into SetOption(), but still exporting it as SetGlobalOption() via Lua for the plugins? Then the call can be moved to the calling function and would fit there more. The native value can be obtained with the help of config.DefaultCommonSettings() as it's done in ResetCmd(). The only problem and most probably breaking compatibility then is the fact that the exported plugin function SetGlobalOptionNative() doesn't check for locals any longer.

So the problem was to export it as "Global" with the reference as with > set, while > set can be used to change locals too. :(

Sh**, sometimes it could be easier...

Edit: This is anyway something which could be done separately, if we really plan to do something in that direction. So far we prevent rewriting the settings.