zyedidia / micro

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

Color scheme issue #3455

Open mdziczkowski opened 2 months ago

mdziczkowski commented 2 months ago

Environment

Issue

Taken steps

Expected result

Apply of the colorscheme

Actual result

Error message:

The colorscheme is invalid

even if the .json file syntax is correct

JoeKar commented 2 months ago
  • created a colorscheme json file [...] even if the .json file syntax is correct

micro's color schemes aren't any json files. They are called [NAME].micro. See colors.md and colorschemes.

mrvruj commented 2 months ago

I have had a colorscheme working for a long time, recently broken. It is in a folder in .../micro/plug/colors/colorscheme-16.micro. When I launch micro, it yells at me "colorscheme-16 is not a valid colorscheme". When I open micro and press ctrl-e, set colorscheme colorscheme-16, it works properly for that instance. When I close micro and open again, the same issue arises.

Andriamanitra commented 2 months ago

I was able to reproduce the issue by having a plugin set up like this:

-- ~/.config/micro/plug/asd/asd.lua
local config = import("micro/config")

function preinit()
    config.AddRuntimeFile("asd", config.RTColorscheme, "asd.micro")
end
# ~/.config/micro/plug/asd/asd.micro
color-link cursor-line "#FF00FF"

It seems that the issue is caused by micro attempting to load colorschemes before the plugin's preinit function runs to initialize the runtime files. I'm not sure if loading a colorscheme to use on startup from plugin ever worked: I tried out release versions v2.0.10-v2.0.14 and got the same asd is not a valid colorscheme error in all of them. Switching to the colorscheme after startup works as expected.


@mrvruj I have had a colorscheme working for a long time, recently broken. It is in a folder in .../micro/plug/colors/colorscheme-16.micro. When I launch micro, it yells at me "colorscheme-16 is not a valid colorscheme". When I open micro and press ctrl-e, set colorscheme colorscheme-16, it works properly for that instance. When I close micro and open again, the same issue arises.

For now you can get around this by placing the colorscheme file directly in ~/.config/micro/colorschemes/colorscheme-16.micro instead of loading it from a plugin.

JoeKar commented 2 months ago

It seems that the issue is caused by micro attempting to load colorschemes before the plugin's preinit function runs to initialize the runtime files. I'm not sure if loading a colorscheme to use on startup from plugin ever worked: I tried out release versions v2.0.10-v2.0.14 and got the same asd is not a valid colorscheme error in all of them. Switching to the colorscheme after startup works as expected.

Yes, you're right, since the order of... https://github.com/zyedidia/micro/blob/5428b3fda2457da08ca10e27a7b6470d7692c474/cmd/micro/micro.go#L322-L330 ...definitely affects this behavior, but it's just half of the truth, because we validate every option already in the moment we parse the settings (https://github.com/zyedidia/micro/commit/4663927098334ae513a2b1d5bfa121905d034167) to be sure to work with fully valid settings and in the moment of validating the colorscheme settings we're still far away of adding the scheme via the plugin. So someone could indeed call it a regression, but this scenario is then hard to fix due to circle dependencies...at least at first sight.

For now you can get around this by placing the colorscheme file directly in ~/.config/micro/colorschemes/colorscheme-16.micro instead of loading it from a plugin.

Yes, I think this is and should be the common approach to load colorschemes. Doing this via plugins doesn't work right now (any longer).

aeadio commented 2 months ago

I just want to say that this appears as a regression in some existing popular color schemes, which were already loading via plugins, such as https://github.com/KiranWells/micro-nord-tc-colors. With this change, it seems to become impossible to distribute color schemes as plugins now?

Can I also make a recommendation to disable the error/warning on startup entirely?

The color scheme is set in settings.json, which I store in my dotfiles repo that I pull down on any new machine(s) I'm working on. I imagine this is a common workflow. I gitignore plugins, because those are separately versioned and may differ per workflow. So when pulling down my config on a new machine, I'm hit with an error on every startup, until I install the associated color scheme / plugin.

On the other hand, if a color scheme is failing to load or is not installed, it should be readily apparent to the user as soon as they see the editor. The warning on startup is not actionable. It's just a nag. Nags are categorically not good UX design.

JoeKar commented 2 months ago

With this change, it seems to become impossible to distribute color schemes as plugins now?

Yes, but due to the design of micro it was never the best idea to mix them up together...it over-complicates a lot of things now.

Can I also make a recommendation to disable the error/warning on startup entirely?

I don't think that it's a wise choice to "ignore" some kind of inconsistency in the config.

The color scheme is set in settings.json, which I store in my dotfiles repo [...] pulling down my config on a new machine, I'm hit with an error on every startup, until I install the associated color scheme / plugin.

Yep, that is the current situation. But there is at least one way to bypass this with your dotfiles too. Why don't you maintain your own .config/micro/colorschemes/ folder in your dotfiles repository, where you store the colorschemes the settings of micro pointing to?

On the other hand, if a color scheme is failing to load or is not installed, it should be readily apparent to the user as soon as they see the editor.

Try to imagine some very simple colorschemes where only a few details should be highlighted, which could be overseen at the first sight.

We can try to handle this "regression", but I find it difficult to prioritize.

@dmaluka: What's your opinion about this? Seems we've unintentionally increased the plugin vs colorscheme mess.

aeadio commented 2 months ago

Try to imagine some very simple colorschemes where only a few details should be highlighted, which could be overseen at the first sight.

But a stop-the-world style interruption at startup feels like a drastic overstep for a pretty niche case. Why doesn't this display as, say, a warning in the gutter after startup? The worst case being solved for here (a color scheme failed to load) is pretty benign, versus the jarring experience of the entire editor stopping in its tracks. It fails the principle of least surprise.

dmaluka commented 2 months ago

I just want to say that this appears as a regression in some existing popular color schemes, which were already loading via plugins, such as https://github.com/KiranWells/micro-nord-tc-colors.

Hmm, this plugin calls config.AddRuntimeFile() from its top-level code, not from init() or preinit() callbacks. Very smart (no). I believe this way of using micro functions from lua was never guaranteed to work, never documented, it works by chance (AddRuntimeFile() happens to be implemented in such a way that it doesn't touch any data structures that are initialized after loading plugins). However, this explains the miracle: as @Andriamanitra correctly noticed, loading colorschemes via plugins at startup in a correct way (i.e. from init() or preinit()) never really used to work, however loading colorschemes via this plugin with this trick used to work until 2.0.14.

I wonder how many other plugins use this trick.

...Actually I've just found out that this trick is not something the plugin author came up with. This trick was suggested by @zyedidia: https://github.com/KiranWells/micro-nord-tc-colors/issues/2#issuecomment-748656296

More and more interesting.

BTW even before 2.0.14 it wasn't fully working: loading colorscheme specified in settings.json worked, however loading colorscheme specified in micro command line (micro -colorscheme nord-16 foo.txt) didn't. (Whereas colorschemes loaded normally from ~/.config/micro/colorschemes, not via plugins, worked and still work successfully in both cases.)

dmaluka commented 2 months ago

On the other hand, if a color scheme is failing to load or is not installed, it should be readily apparent to the user as soon as they see the editor.

No, it is not apparent to the user why the colorscheme was not applied. foo is not a valid colorscheme at least indicates that micro has successfully read the foo setting from settings.json, before it failed to apply this setting.

But a stop-the-world style interruption at startup feels like a drastic overstep for a pretty niche case. Why doesn't this display as, say, a warning in the gutter after startup?

There may be multiple configuration errors at startup. With a "stop-the-world interruption" we can report them all to the user, one by one, whereas with a warning in the gutter we would only be able to show one of them.

...Let's think how to address the root issue (non-working colorschemes loading via plugins at startup, in particular with existing plugins), then we won't need to think how to hide its consequences.

mdziczkowski commented 1 month ago

I think that all micro documentation should be included to each micro release instead beeing avaliable only online.

Besides of above, your don't address setting colors for specific languages (for example: python, perl, etc.) 😜

JoeKar commented 1 month ago

I think that all micro documentation should be included to each micro release instead beeing avaliable only online.

It isn't just available online only as mentioned here & here: > help [USE_TAB_TAB_HERE]

By tabbing after > help you will receive the available list of help pages. When you've the desired entry press Enter. The documentation is built into the binary too...as many other stuff. :wink: