solarized-emacs icon indicating copy to clipboard operation
solarized-emacs copied to clipboard

Avoid side effects on install/build

Open colonelpanic8 opened this issue 5 years ago • 7 comments

Solarized is one of the only themes that seems to have side effects that somehow result from an update/install. I haven't had time to investigate why this is, but this should probably be avoided.

colonelpanic8 avatar Feb 14 '19 23:02 colonelpanic8

What side effect you seen? You tell me more information, I can try work on it.

conao3 avatar Nov 02 '19 19:11 conao3

It applied facedefs while byte compiling/loading the -theme files, idk if it was fixed or not.. Not really a big practical issue. I havent seen it myself for many years IIRC, I don't know what you have to do to experience it as a problem.

(let ((custom--inhibit-theme-enable nil)) could potentially have brought it back if it was fixed, I don't really understand what the documentation of that variable implies.

thomasf avatar Nov 03 '19 05:11 thomasf

I encountered the same problem, run the code snippet below:

(require 'solarized-theme)
(load-theme 'solarized-dark-high-contrast nil t)

The solarized-dark-high-contrast theme will pollution my current emacs theme, even if i asked it just load and not enable.

I am writing a theme switch tool which loaded all themes but didn't enable any one, so i can generate a menu with theme name and documentation information, so the user can select one to enable, it looks like bellow:

afternoon                                Dark color theme with a deep blue background
brin                                     Space Grey theme for Emacs
dorsey                                   A dark grunge color theme
fogus                                    A bluecolor theme
graham                                   A dark color theme for Emacs
tango-dark                               Face colors using the Tango palette (dark background).
tango                                    Face colors using the Tango palette (light background).
tsdh-dark                                A dark theme used and created by Tassilo Horn.
tsdh-light                               A light Emacs theme.
wheatgrass                               High-contrast green/blue/brown faces on a black background.
whiteboard                               Face colors similar to markers on a whiteboard.
wombat                                   Medium-contrast faces with a dark gray background.

Many popular themes are works well, except solarized-theme, all it's themes not support load and not enable behavior.

tangxinfa avatar Dec 03 '19 03:12 tangxinfa

When you run load-theme you end up running a lot of custom-set-faces and custom-set-variables. Therefore, the effect will remain if the theme that is subsequently activated does not successfully overwrite the value set by solarized. This is an Emacs specification.

Now, the original issue was that there were side effects when update/install (maybe at byte-compiling). The thread owner and you reported are different.

conao3 avatar Dec 03 '19 09:12 conao3

I have a feeling that (let ((custom--inhibit-theme-enable nil)) prohibits the NO-ENABLE argument of load-theme to work correctly (load-theme THEME &optional NO-CONFIRM NO-ENABLE). I'm not sure if this is currently easy to resolve given the way child themes currently works.

It seems like we could change the order of child theme and base solarized face defs are run but I'm not sure if that causes other unwanted scenarios instead (base anc child facesdes not being merged?).

I have a feeling that the proper solution would be to write code that merges custom-theme-set-faces and custom-theme-set-variables section from child theme and parent before they are actually executed.

I am not sure I want to make invasive changes to how that works right now since we just change it recently so if we are going to resolve it now it has to be compatible with how child themes are set up now.

thomasf avatar Dec 03 '19 15:12 thomasf

I encountered the same problem, run the code snippet below:

(require 'solarized-theme)
(load-theme 'solarized-dark-high-contrast nil t)

The solarized-dark-high-contrast theme will pollution my current emacs theme, even if i asked it just load and not enable.

I am writing a theme switch tool which loaded all themes but didn't enable any one, so i can generate a menu with theme name and documentation information, so the user can select one to enable, it looks like bellow:

afternoon                                Dark color theme with a deep blue background
brin                                     Space Grey theme for Emacs
dorsey                                   A dark grunge color theme
fogus                                    A bluecolor theme
graham                                   A dark color theme for Emacs
tango-dark                               Face colors using the Tango palette (dark background).
tango                                    Face colors using the Tango palette (light background).
tsdh-dark                                A dark theme used and created by Tassilo Horn.
tsdh-light                               A light Emacs theme.
wheatgrass                               High-contrast green/blue/brown faces on a black background.
whiteboard                               Face colors similar to markers on a whiteboard.
wombat                                   Medium-contrast faces with a dark gray background.

Many popular themes are works well, except solarized-theme, all it's themes not support load and not enable behavior.

You can probably work around the issue at by unloading the theme again after loading it. IIRC that removes any faces applied during the load theme phase regardless of how it's set. It will probably still flash new faces temporarily, maybe it's possible to inhibit fontification (or whatever applies faces to displays) while loading/unloading to hide visual side effects .

thomasf avatar Dec 04 '19 06:12 thomasf

I have a feeling that (let ((custom--inhibit-theme-enable nil)) prohibits the NO-ENABLE argument of load-theme to work correctly (load-theme THEME &optional NO-CONFIRM NO-ENABLE). I'm not sure if this is currently easy to resolve given the way child themes currently works.

It seems like we could change the order of child theme and base solarized face defs are run but I'm not sure if that causes other unwanted scenarios instead (base anc child facesdes not being merged?).

I have a feeling that the proper solution would be to write code that merges custom-theme-set-faces and custom-theme-set-variables section from child theme and parent before they are actually executed.

I am not sure I want to make invasive changes to how that works right now since we just change it recently so if we are going to resolve it now it has to be compatible with how child themes are set up now.

@thomasf You are right, the sentence (let ((custom--inhibit-theme-enable nil)) in solarized.el prohibits the NO-ENABLE argument of load-theme to work correctly.

tangxinfa avatar Dec 05 '19 03:12 tangxinfa