Add toggle & togglelocal command #3783

cutelisp · 2025-06-24T19:03:54Z

This adds toggleandtogglelocal` command as requested here #2086.
Not sure if this is the most correct approach, I found settings code a bit confusing and with non-intuitive variable names and behaviors.
Feedback is appreciated.

internal/action/command.go

dmaluka · 2025-06-24T23:07:46Z

runtime/help/commands.md

 * `setlocal 'option' 'value'`: sets the option to value locally (only in the
   current buffer). This will *not* modify `settings.json`.
+
+* `toggle 'option'`: toggles the option to `on/off`. See the `options` help


Just "toggles the option on/off"?

dmaluka · 2025-06-24T23:15:40Z

runtime/help/commands.md

   current buffer). This will *not* modify `settings.json`.
+
+* `toggle 'option'`: toggles the option to `on/off`. See the `options` help
+  topic for a list of options you can toggle, only works with `on/off` options. This will modify your


The options help refers to their values mostly as true and false, not on and off... Instead of this entire sentence "See the...", maybe just "Only works with boolean options"?

Seems good, I'll change it.

Your comment made me wonder, maybe we could improve the toggle suggestions - i.e. when the user types toggle and hit tab the options that show up as suggestions are only boolean options. You think it would be a good idea? Maybe it's a problem for another PR, it could be done also for setlocal.

Surely sounds like a good idea.

Side note: I've just tried using setlocal for a global-only option (infobar), and (besides the fact that it shows up among suggestions), the error displayed after setlocal infobar off is: Invalid option. It would be nice to make it more informative / less confusing, to make it clear why it is invalid in this case.

Side note 2: I've just taken a glance at the code in OptionValueComplete(), what a mess.

Ye, I got confused plenty of times as well with settings code in general.
Starting with, a setting can be global or local but then we have GlobalSettings map[string]interface{} at first sight I'd say its a list of global-only settings, but it's, in fact, a list of all settings. Inside there's a list of settings that should never be globally modified. Why not call it just Settings or AllSettings?

Surely sounds like a good idea.

Thinking in advance I could already create a function isBooleanOption(opt) booleanto use on toggleCmd & toggleLocalCmd and in the future on the suggestion logic, wt do you think?

I'm not sure what for.

runtime/help/commands.md

cutelisp · 2025-06-29T09:27:22Z

Made changes base on your feedback.
I am wondering is "boolean option" the most user friendly name?

internal/action/command.go

runtime/help/commands.md

internal/action/command.go

dmaluka · 2025-06-29T10:31:46Z

Made changes base on your feedback.

Thanks. It's better to squash them into the first commit. Fortunately github is capable of displaying diffs between force-pushes (even though is not as good at that as gerrit, for example), so there is little value in keeping them in a separate commit, it only clutters the commit history. Splitting changes into separate commits is useful not in such cases, but in cases when those changes are actually logically separate, so that splitting them actually makes review easier (for example, changing the order of the existing options in the documentation, which you've done, would make sense to do in a separate commit).

I am wondering is "boolean option" the most user friendly name?

You mean that not all users are programmers and know what "boolean" is? Well, OTOH it sounds the least mouthful... We might clarify it a bit, e.g.: "...Only works with boolean options, i.e. options with values true and false." OTOH we are already using this term in a couple of other places in the documentation...

cutelisp · 2025-06-29T10:41:35Z

You mean that not all users are programmers and know what "boolean" is?

Yes

OTOH we are already using this term in a couple of other places in the documentation...

That may answer my question, maybe "boolean" is indeed friendly

dmaluka · 2025-06-29T11:39:27Z

runtime/help/commands.md


 * `setlocal 'option' 'value'`: sets the option to value locally (only in the
   current buffer). This will *not* modify `settings.json`.
+


Just noticed (thanks to git show): unneeded trailing spaces added here.

runtime/help/commands.md

dmaluka · 2025-06-29T12:13:57Z

runtime/help/commands.md

+   This will modify your `settings.json` with the new value.
+
+* `togglelocal 'option'`: toggles the option true/false locally
+   (only in the current buffer). Only works with boolean options.


nit: I'd split the lines this way, to keep the lines width more or less the same as in the surroundings (i.e. not too wide, not too narrow):

diff --git a/runtime/help/commands.md b/runtime/help/commands.md index ad5ba7db..fde3a4e0 100644 --- a/runtime/help/commands.md +++ b/runtime/help/commands.md @@ -72,12 +72,12 @@ quotes here but these are not necessary when entering the command in micro. * `setlocal 'option' 'value'`: sets the option to value locally (only in the current buffer). This will *not* modify `settings.json`. -* `toggle 'option'`: toggles the option true/false. Only works with boolean options. - This will modify your `settings.json` with the new value. +* `toggle 'option'`: toggles the option true/false. Only works with boolean + options. This will modify your `settings.json` with the new value. -* `togglelocal 'option'`: toggles the option true/false locally - (only in the current buffer). Only works with boolean options. - This will *not* modify `settings.json`. +* `togglelocal 'option'`: toggles the option true/false locally (only in the + current buffer). Only works with boolean options. This will *not* modify + `settings.json`. * `reset 'option'`: resets the given option to its default value

dmaluka · 2025-06-29T19:55:07Z

runtime/help/commands.md

+* `toggle 'option'`: toggles the option true/false. Only works with boolean
+   options. This will modify your `settings.json` with the new value.
+
+* `togglelocal 'option'`: toggles the option true/false locally(only in the


nit: missing space before parenthesis. :)

cutelisp · 2025-07-11T15:15:21Z

@dmaluka lacking anything?

dmaluka · 2025-07-11T21:09:22Z

@dmaluka lacking anything?

Last time I checked, just this comment: #3783 (comment) :)

cutelisp · 2025-07-11T21:22:16Z

Last time I checked, just this comment: #3783 (comment) :)

I've read #3783 (comment) and force pushed a patched version (I thought). I wonder how I failed to add a space. My bad 😞

JoeKar · 2025-07-15T17:28:48Z

internal/action/command.go

+	curValBool, ok := curVal.(bool)
+	if !ok {
+		InfoBar.Error("Not a boolean option")
+		return
+	}


I can remember #3021 (comment) 😉
Is there really the need to limit the (local) toggling to boolean options, since we could cycle through the other as well?

cycle through the other

What other means here? Every option?

Every option isn't possible, since we've integers and custom strings as well, but OptionChoices and colorscheme are additionally toggle-able...at least from my perspective.

Erm, from my POV, a toggle would be on/off. I think the only non-boolean toggleable options are fileformat and helpsplit from OptionChoices.

Aside from naming, I don't think creating a cycling command would be useful. We could make options with exactly two values toggleable, though.

Going further, we could enhance RegisterCommonOption and RegisterGlobalOption to accept option choices directly (and other verifications?).

Cycling through colorschemes with a command/keybinding might be nice but I agree that wouldn't really be called "toggling" any more.

Just out of curiosity, what's the use case?

Cycling through all the colorschemes is something you do occasionally to either pick the one you like the best or to check that they all work as expected. I know it can be done in a plugin but having it built-in could help new users pick a colorscheme.

I'm not sure if it is worth implementing, but if it is I definitely think it shouldn't be part of the toggle command.

Ohh I see.

could help new users pick a colorscheme.

Surely, selecting 1 by 1 is a PITA. But in this case I think a colorscheme preview suits more into this problem. It was already requested #1782 #2677

I can remember #3021 (comment) 😉

Actually prior to that comment there was this discussion: #2876 (comment)

starting from this comment of mine:

One note though: perhaps it's better to go a simpler route and, instead of this "generic and extensible" matchbracestyle option with highlight and underline values, add a simple boolean option e.g. matchbracehighlight? An advantage is that, if we implement a generic support for easy toggling on/off arbitrary options (see #2086), then it will be readily usable for this matchbracehighlight option as well, to easily toggle between highlighting and underlining.

And now, I tend to agree with @cutelisp and @Andriamanitra that toggle for options with 3 or more choices would be rather weird [*]. But, for options with exactly 2 choices, like this matchbracestyle, it seems like a nice idea indeed. And that would also address precisely the concern from my quoted comment above.

[*] Even though, as I've realized just now, it was me who suggested that stupid idea, in #2876 (comment).

I'll give it a try on including "2-choice options"

Ok, you guys convinced me.
We give it a try for the "2-choice options". 👍

cutelisp · 2025-07-17T19:15:09Z

Added support for "2-choice options"

cutelisp · 2025-07-17T19:17:23Z

internal/action/command.go

+		curVal = config.GetGlobalOption(option)
+	}
+	if curVal == nil {
+		return config.ErrInvalidOption


Should we bother saying it's an invalid option or should we just say it's not a toggleable option anyway.

At this position it is invalid.

The point is what is invalid? An invalid option or a invalid option for this command?

For example: setlocal autosave 0 -> invalid option
Autosave is indeed a valid option but it's not a local option.

I was willing to improve error messages that's why I created ErrOptNotToggleable.

At this position it is invalid.

Note that at this point it might be a valid option yet a global-only.

To improve accuracy in this command and other parts of the code, we could encapsulate the logic into dedicated functions, something like this:

func GetGlobalOption*(name string) (interface{}, error) { if v, ok := GlobalSettings[name]; ok { return v, nil } return nil, ErrInvalidOption } func (b *Buffer) GetOption(name string) (interface{}, error) { if v, ok := b.Settings[name]; ok { return v, nil } if _, err := GetGlobalOption(name); err != nil { return nil, err } return nil, ErrNotLocalOpt }

What do you think?

The point is what is invalid? An invalid option or a invalid option for this command?

When toggleOption() is called with local set to false and calls config.GetGlobalOption(option) with a option not tracked within, then it is indeed an ErrInvalidOption. If it is toggleable will be checked later on.

GetGlobalOption() is already exported to plugins:

micro/cmd/micro/initlua.go

Line 90 in dbdf753

ulua.L.SetField(pkg, "GetGlobalOption", luar.New(ulua.L, config.GetGlobalOption))

GetGlobalOption() is already exported to plugins:

Yes, and used on go side as well. Thats why GetGlobalOption*, it would be a different function.
Note that we already use this logic.

if _, ok := config.GlobalSettings[option]; !ok { return config.ErrInvalidOption }

When toggleOption() is called with local set to false and calls config.GetGlobalOption(option) with a option not tracked within, then it is indeed an ErrInvalidOption. If it is toggleable will be checked later on.

I am aware, the edge case comes when we call it with local set to true and we give a valid global-only option.
(b *Buffer) GetOption() would handle it with a specific error

cutelisp · 2025-07-17T19:31:08Z

internal/action/command.go

+	if !local {
+		if err := SetGlobalOptionNative(option, newVal); err == nil {
+			return nil
+		}
+	}
+
+	if err := h.Buf.SetOptionNative(option, newVal); err != nil {
+		return err
+	}


I took this logic from SetCmd().

err := SetGlobalOption(option, value) if err == config.ErrInvalidOption { err := h.Buf.SetOption(option, value) if err != nil { InfoBar.Error(err) } } else if err != nil { InfoBar.Error(err) }

But now I wonder if making this fallback makes sense at all?

SetGlobalOption is just a SetGlobalOptionNative wrapper.
SetGlobalOptionNative have a fallback for local settings itself.

micro/internal/action/command.go

Lines 636 to 641 in 3a7705a

// check for local option first...

for _, s := range config.LocalSettings {

if s == option {

return MainTab().CurPane().Buf.SetOptionNative(option, nativeValue)

}

}

Maybe I am missing something but I don't see a way where config.ErrInvalidOption is returned and makes sense to call Buf.SetOption*()

Indeed, looks like we can simplify that and just switch based on if !local {} without fallback.

Right.
As I pointed here SetGlobalOptionNative has some odd behavior. I just realized that, with the current logic, setting a global-only value it also populates the Settings field of each buffer in openBuffers. IMO, the current variable and function names related to settings are misleading, and we should consider renaming them if we eventually patch these functions.

cutelisp · 2025-07-25T03:26:16Z

The error logic is consistent with other commands, so I'll keep things as they are and probably create a PR to improve error handling across commands.
It seems Setcmd() might need a small patch (#3783 (comment)), but let's leave that for another PR as well.
Lacking something to merge, @dmaluka?

dmaluka · 2025-08-04T22:40:36Z

internal/action/command.go

+		return config.ErrOptNotToggleable
+	}
+
+	if !local {


nit: flip the order of the if/else branches (just to keep it the same as at the beginning of the function)?

jwatt · 2025-09-04T11:53:37Z

It looks like this stalled out with one last nit to address? Any chance it could land without that? Thanks for your work on this @cutelisp and reviewers.

cutelisp · 2025-09-04T15:57:46Z

Sorry for delay.

jwatt · 2025-09-06T15:08:22Z

Thanks everyone. :-)

dmaluka reviewed Jun 24, 2025

View reviewed changes

dmaluka reviewed Jun 29, 2025

View reviewed changes

internal/action/command.go Outdated Show resolved Hide resolved

runtime/help/commands.md Outdated Show resolved Hide resolved

internal/action/command.go Outdated Show resolved Hide resolved

cutelisp force-pushed the toggleCommand branch 2 times, most recently from cc5927c to 0046b74 Compare June 29, 2025 11:16

dmaluka reviewed Jun 29, 2025

View reviewed changes

cutelisp force-pushed the toggleCommand branch from 0046b74 to 8de507c Compare June 29, 2025 11:58

dmaluka reviewed Jun 29, 2025

View reviewed changes

cutelisp force-pushed the toggleCommand branch from 8de507c to bbb8301 Compare June 29, 2025 13:00

dmaluka reviewed Jun 29, 2025

View reviewed changes

cutelisp force-pushed the toggleCommand branch from bbb8301 to 9a8ef03 Compare June 29, 2025 21:07

cutelisp mentioned this pull request Jul 8, 2025

Enhance Tab Bar #3795

Open

cutelisp force-pushed the toggleCommand branch from 9a8ef03 to f132e94 Compare July 11, 2025 21:17

dmaluka approved these changes Jul 11, 2025

View reviewed changes

JoeKar reviewed Jul 15, 2025

View reviewed changes

cutelisp force-pushed the toggleCommand branch from f132e94 to e2b928e Compare July 17, 2025 19:13

cutelisp commented Jul 17, 2025

View reviewed changes

cutelisp force-pushed the toggleCommand branch from e2b928e to 217ef4a Compare July 25, 2025 03:09

JoeKar approved these changes Aug 4, 2025

View reviewed changes

dmaluka reviewed Aug 4, 2025

View reviewed changes

cutelisp force-pushed the toggleCommand branch from 217ef4a to 160c13a Compare September 4, 2025 15:52

Add toggle & togglelocal command

595c0cf

cutelisp force-pushed the toggleCommand branch from 160c13a to 595c0cf Compare September 4, 2025 15:56

JoeKar approved these changes Sep 5, 2025

View reviewed changes

JoeKar merged commit 0b9c7c0 into zyedidia:master Sep 5, 2025
6 checks passed


		* `setlocal 'option' 'value'`: sets the option to value locally (only in the
		current buffer). This will not modify `settings.json`.

	// check for local option first...
	for _, s := range config.LocalSettings {
	if s == option {
	return MainTab().CurPane().Buf.SetOptionNative(option, nativeValue)
	}
	}

Add toggle & togglelocal command #3783

Add toggle & togglelocal command #3783

Conversation

cutelisp commented Jun 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cutelisp commented Jun 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmaluka commented Jun 29, 2025

Uh oh!

cutelisp commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cutelisp commented Jul 11, 2025

Uh oh!

dmaluka commented Jul 11, 2025

Uh oh!

cutelisp commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cutelisp commented Jul 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cutelisp Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

cutelisp commented Jun 29, 2025 •

edited

Loading

cutelisp Jul 21, 2025 •

edited

Loading

cutelisp Jul 20, 2025 •

edited

Loading