child themes does not work (they do work now. still question that needs to be resolved)

[thomasf]

It does not seem that emacs want to redefine a face in a theme after its initially been set. It's quite easy to test, the included solarized-wombat-dark-theme.el sets inactive mode line to a blackish color and it does not do that now.

The patch below fixes it for me but I'm not sure. Moving the let binding to somewhere where it's actually called for all themes (solarized-definition?) might also work.

The documentation for custom--inhibit-theme-enable says:

custom--inhibit-theme-enable is a variable defined in ‘custom.el’.
Its value is nil

Documentation:
Whether the custom-theme-set-* functions act immediately.
If nil, ‘custom-theme-set-variables’ and ‘custom-theme-set-faces’
change the current values of the given variable or face.  If
non-nil, they just make a record of the theme settings.

It's a bit unclear to me what other implication this has for the theme system in general if we disable it.

diff --git a/solarized-faces.el b/solarized-faces.el
index f2eee33..dd56e62 100644
--- a/solarized-faces.el
+++ b/solarized-faces.el
@@ -12,6 +12,9 @@ customize the resulting theme."
 ;;; Color palette
   (eval
    `(solarized-with-color-variables ',variant ,color-palette
+;;; Child theme Faces
+      ,(when childtheme
+         `(funcall ',childtheme))
 ;;; Theme Faces
       (custom-theme-set-faces
        ',theme-name
@@ -2066,8 +2069,7 @@ customize the resulting theme."
        `(xterm-color-names-bright [,base03 ,orange ,base01 ,base00
                                            ,base0 ,violet ,base1 ,base3]))
 ;;; Setup End
-      ,(when childtheme
-         `(funcall ',childtheme)))))
+      )))

 (provide 'solarized-faces)
diff --git a/solarized.el b/solarized.el
index 3bb2421..1529232 100644
--- a/solarized.el
+++ b/solarized.el
@@ -417,8 +417,7 @@ If OVERWRITE is non-nil, overwrite theme file if exist."
               `((require 'solarized)
                 (deftheme ,theme-name
                   ,(format "The %s colour theme of Solarized colour theme flavor." theme-name))
-                (let ((custom--inhibit-theme-enable nil))
-                  (solarized-create-theme-with-palette ',variant ',theme-name ',core-palette ',childtheme))
+                (solarized-create-theme-with-palette ',variant ',theme-name ',core-palette ',childtheme)
                 (provide-theme ',theme-name)
                 (provide ',(intern (format "%s-theme" theme-name)))))))
     path))

[conao3]

First, I think child-theme is not a good idea for the user customization scheme. Because of the child-theme mechanism existing, solarized cannot enable the lexical-binding required for modern packages.

It is better to change the current solarized-with-color-variables to with-solarized-color-variables, and the upper function should be designed to accept child theme as normal sexp instead of function. This is a bit painful, but it is an incompatibility necessary to review it for a better design.

[thomasf]

It probably hasn't worked in years other than it just accepts the function and then Emacs does nothing unless the user is defining faces that the main theme don't have. This functionality has probably phased out itself by not working for a very long time so fixing it is maybe not the best option, thats why I created an issue instead of doing something about it.

[thomasf]

The question is, do we just hard break users by removing the argument or just log a warning with a reference to how it should be used if someone tries to use it?

[conao3]

OK, I try compose my proposal.

---

My proposal includes change macro name (solarized-with-color-variables to with-solarized-color-variables) so if the user uses the old name, the user gets an error or warning. But solarized-theme provides a theme file only. Other function and macros is all internal function.

[thomasf]

related https://github.com/bbatsov/solarized-emacs/issues/325