theme and a question about creating them

theme and a question about creating them

[Dirk-Jan C. Binnema]

[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]

Hi all,

Find attached a theme inspired by zenburn-theme[1]; it's only the things I
could generate with the nice GNU Emacs Theme Creator. To adapt the complete
theme (which was written for color-theme.el) to the new system, it'd be nice
if some abstraction were possible i.e., define a handful of base colors, and
re-use them; the original zenburn-theme is /huge/, so it's a bit cumbersome to
work with literals everywhere.

I tried to use defvar/defconst and :inherit do this (as the old theme did),
but it does not seem to work in the new system, i.e.. I tried something like:

(defconst mycolor "#123456")
(deftheme mytheme)
(custom-theme-set-faces 'mytheme
     `(default ((t (:background ,mycolor))))) 

but it seems the defconst is not picked up. And for :inherit, it seems you
cannot refer to the faces defined in the same 'custom-theme-set-faces' form.
I could define theme elsewhere of course, but obviously it'd be nice if my
theme file would be self-contained.

Is there some way to use constants like this, either with defconst/defvar
or :inherit?

Thanks,
Dirk.

[1] http://www.emacswiki.org/emacs/ColorThemeZenburn


[-- Attachment #2: zenburn-theme.el --]
[-- Type: text/plain, Size: 2338 bytes --]

;;; zenburn-theme.el --- Custom face theme for Emacs

;; Copyright (C) 2010 Dirk-Jan C. Binnema.

;; This file is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; This file is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.

;;; Code:

(deftheme zenburn
  "Based on the Zenburn theme.")

(custom-theme-set-faces
 'zenburn
 '(default ((t (:background "#3f3f3f" :foreground "#dcdccc"))))
 '(cursor ((t (:background "#e0cf9f" :foreground "#ffffff"))))
 '(region ((t (:background "#5f5f5f"))))
 '(mode-line ((t (:background "#1e2320" :foreground "#acbc90"))))
 '(mode-line-inactive ((t (:background "#2e3330" :foreground "#88b090"))))
 '(fringe ((t (:background "#464646"))))
 '(minibuffer-prompt ((t (:foreground "#e0cf9f" :weight bold))))
 '(font-lock-builtin-face ((t (:foreground "#8cd0d3" :weight bold))))
 '(font-lock-comment-face ((t (:slant italic :foreground "#999999"))))
 '(font-lock-constant-face ((t (:foreground "#dca3a3" :weight bold))))
 '(font-lock-function-name-face ((t (:foreground "#8cd0d3"))))
 '(font-lock-keyword-face ((t (:foreground "#e0cf9f" :weight bold))))
 '(font-lock-string-face ((t (:foreground "#cc9393"))))
 '(font-lock-type-face ((t (:foreground "#dfdfbf" :weight bold))))
 '(font-lock-variable-name-face ((t (:foreground "#e0cf9f"))))
 '(font-lock-warning-face ((t (:background "#332323" :foreground "#e37170" :weight bold))))
 '(isearch ((t (:background "#506070" :weight bold))))
 '(lazy-highlight ((t (:background "#1e2320"))))
 '(link ((t (:foreground "#e0cf9f" :underline t))))
 '(link-visited ((t (:foreground "#dfaf8f" :underline t))))
 '(button ((t (:background "#506070" :foreground "#e0cf9f" :weight bold))))
 '(header-line ((t (:background "#2e3330" :foreground "#88b090")))))

(provide-theme 'zenburn)

;; Local Variables:
;; no-byte-compile: t
;; End:

;;; zenburn-theme.el  ends here

[-- Attachment #3: Type: text/plain, Size: 170 bytes --]


-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

Re: theme and a question about creating them

[Chong Yidong]

Dirk-Jan C. Binnema <djcb.bulk@gmail.com> writes:

> I tried to use defvar/defconst and :inherit do this (as the old theme did),
> but it does not seem to work in the new system, i.e.. I tried something like:
>
> (defconst mycolor "#123456")
> (deftheme mytheme)
> (custom-theme-set-faces 'mytheme
>      `(default ((t (:background ,mycolor)))))
>
> but it seems the defconst is not picked up. And for :inherit, it seems you
> cannot refer to the faces defined in the same 'custom-theme-set-faces' form.
> I could define theme elsewhere of course, but obviously it'd be nice if my
> theme file would be self-contained.
>
> Is there some way to use constants like this, either with defconst/defvar
> or :inherit?

Currently, the theme loading code is too strict about the forms in the
theme file that it will evaluate.  It uses `unsafep' to check the forms,
and (defconst mycolor "#123456") is considered unsafe under the criteria
used by `unsafep'.

I am not sure what's the best way of handling this.  Maybe we should
just go back to unconditionally loading theme files, since it's unlikely
we can make evaluating Elisp code truly safe.  Any objections or
alternative ideas?

Re: theme and a question about creating them

[Yavuz]

Chong Yidong <cyd <at> stupidchicken.com> writes:

> I am not sure what's the best way of handling this.  Maybe we should
> just go back to unconditionally loading theme files, since it's unlikely
> we can make evaluating Elisp code truly safe.  Any objections or
> alternative ideas?
> 

I had run into the same security 'features' a few months back when creating (yet
another) version of zenburn. As a user and designer of color themes, I would
prefer not to have any restrictions, which seem out of place in an editor whose
raison d'être is letting its users do what they want. All .el files are code
(and data) and should be treated equally.

In any case, I have stumbled on a 'solution'; I explicitly load the theme file
in my init file:

(load-file (concat dotfiles-dir "themes/zendust-theme.el"))

Then the defvars for defining colors and :inherit stuff seem to work. I have no
idea if this has unwanted side effects.

My theme is at https://github.com/yarkun/zendust

Re: theme and a question about creating them

[Dirk-Jan C. Binnema]

Hi,

>>>>> On Sun, 2 Jan 2011 09:16:38 -0500, Chong Yidong ("CYD") wrote:

  >> Dirk-Jan C. Binnema <djcb.bulk@gmail.com> writes: I tried to use
  >> defvar/defconst and :inherit do this (as the old theme did), but it does
  >> not seem to work in the new system, i.e.. I tried something like:
  >> 
  >> (defconst mycolor "#123456")
  >> (deftheme mytheme)
  >> (custom-theme-set-faces 'mytheme
  >> `(default ((t (:background ,mycolor)))))
  >> 
  >> but it seems the defconst is not picked up. And for :inherit, it seems
  >> you cannot refer to the faces defined in the same
  >> 'custom-theme-set-faces' form.  I could define theme elsewhere of course,
  >> but obviously it'd be nice if my theme file would be self-contained.
  >> 
  >> Is there some way to use constants like this, either with defconst/defvar
  >> or :inherit?

  CYD> Currently, the theme loading code is too strict about the forms in the
  CYD> theme file that it will evaluate.  It uses `unsafep' to check the
  CYD> forms, and (defconst mycolor "#123456") is considered unsafe under the
  CYD> criteria used by `unsafep'.

  CYD> I am not sure what's the best way of handling this.  Maybe we should
  CYD> just go back to unconditionally loading theme files, since it's
  CYD> unlikely we can make evaluating Elisp code truly safe.  Any objections
  CYD> or alternative ideas?

I'm fine with that, but I'm hardly qualified to see the implications of
this.

Somewhat related, I think that emacs should promote the use of :inherit; many
modes have their own faces which require way to many customizations (look
e.g.. at zenburn which has hundreds of lines).

Finally, load-theme seems like a convenient mechanism to set/unset any set of
values. I wonder if it could be used (if unconditional loading is enabled) to
ship a theme with sets up things in a way that many new users expect, i.e..,
with delete-selection-mode, column-number-mode and a couple of other things.

Best wishes,
Dirk.

-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

Re: theme and a question about creating them

[Stefan Monnier]

>> (defconst mycolor "#123456")
>> (deftheme mytheme)
>> (custom-theme-set-faces 'mytheme
>> `(default ((t (:background ,mycolor)))))

> Currently, the theme loading code is too strict about the forms in the
> theme file that it will evaluate.  It uses `unsafep' to check the forms,
> and (defconst mycolor "#123456") is considered unsafe under the criteria
> used by `unsafep'.

> I am not sure what's the best way of handling this.

How 'bout:

   (deftheme mytheme)
   (let ((mycolor "#123456"))
     (custom-theme-set-faces 'mytheme
     `(default ((t (:background ,mycolor))))))

> Maybe we should just go back to unconditionally loading theme files,
> since it's unlikely we can make evaluating Elisp code truly safe.
> Any objections or alternative ideas?

IIUC the problem is that users tend to expect themes to be "just data",
without any possible risks, so they may try themes they download from
random places on the web.
So, I think it's OK to use unsafep to weed out the "safe" themes, and
prompt users for the remaining themes.  But the prompt should make it
clear that we have no determined the theme to be dangerous, just that
Emacs did not recognize it as obviously safe and is unable to assess
whether it's dangerous or not.


        Stefan

Re: theme and a question about creating them

[Johan Bockgård]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> (defconst mycolor "#123456")
>>> (deftheme mytheme)
>>> (custom-theme-set-faces 'mytheme
>>> `(default ((t (:background ,mycolor)))))
>
>> Currently, the theme loading code is too strict about the forms in the
>> theme file that it will evaluate.  It uses `unsafep' to check the forms,
>> and (defconst mycolor "#123456") is considered unsafe under the criteria
>> used by `unsafep'.
>
>> I am not sure what's the best way of handling this.
>
> How 'bout:
>
>    (deftheme mytheme)
>    (let ((mycolor "#123456"))
>      (custom-theme-set-faces 'mytheme
>      `(default ((t (:background ,mycolor))))))

unsafep does not allow \`

Re: theme and a question about creating them

[Chong Yidong]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Maybe we should just go back to unconditionally loading theme files,
>> since it's unlikely we can make evaluating Elisp code truly safe.
>> Any objections or alternative ideas?
>
> IIUC the problem is that users tend to expect themes to be "just data",
> without any possible risks, so they may try themes they download from
> random places on the web.
>
> So, I think it's OK to use unsafep to weed out the "safe" themes, and
> prompt users for the remaining themes.  But the prompt should make it
> clear that we have no determined the theme to be dangerous, just that
> Emacs did not recognize it as obviously safe and is unable to assess
> whether it's dangerous or not.

Sounds reasonable.

Re: theme and a question about creating them

[Stefan Monnier]

>>>> (defconst mycolor "#123456")
>>>> (deftheme mytheme)
>>>> (custom-theme-set-faces 'mytheme
>>>> `(default ((t (:background ,mycolor)))))
>> 
>>> Currently, the theme loading code is too strict about the forms in the
>>> theme file that it will evaluate.  It uses `unsafep' to check the forms,
>>> and (defconst mycolor "#123456") is considered unsafe under the criteria
>>> used by `unsafep'.
>> 
>>> I am not sure what's the best way of handling this.
>> 
>> How 'bout:
>> 
>> (deftheme mytheme)
>> (let ((mycolor "#123456"))
>> (custom-theme-set-faces 'mytheme
>> `(default ((t (:background ,mycolor))))))

> unsafep does not allow \`

I can't think of a reason why backquote wouldn't be safe, so it sounds
like it's just missing from the list of safe forms.  Or maybe it's the
general handling of macros which is at stake, but in any case it
shouldn't be unsolvable.


        Stefan

Re: theme and a question about creating them

[Johan Bockgård]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> unsafep does not allow \`
>
> I can't think of a reason why backquote wouldn't be safe, so it sounds
> like it's just missing from the list of safe forms.  Or maybe it's the
> general handling of macros which is at stake, but in any case it
> shouldn't be unsolvable.

Something like


2011-01-07  Johan Bockgård  <bojohan@gnu.org>

	* emacs-lisp/unsafep.el (unsafep): Handle backquoted forms.


diff --git a/lisp/emacs-lisp/unsafep.el b/lisp/emacs-lisp/unsafep.el
index a62f8de..5dee2af 100644
--- a/lisp/emacs-lisp/unsafep.el
+++ b/lisp/emacs-lisp/unsafep.el
@@ -202,6 +202,9 @@ UNSAFEP-VARS is a list of symbols with local bindings."
 	      (dolist (x (nthcdr 3 form))
 		(setq reason (unsafep-progn (cdr x)))
 		(if reason (throw 'unsafep reason))))))
+       ((eq fun '\`)
+	;; Backquoted form - safe if its expansion is.
+	(unsafep (cdr (backquote-process (cadr form)))))
        (t
 	;;First unsafep-function call above wasn't nil, no special case applies
 	reason)))))

Re: theme and a question about creating them

[Stefan Monnier]

>>> unsafep does not allow \`
>> I can't think of a reason why backquote wouldn't be safe, so it sounds
>> like it's just missing from the list of safe forms.  Or maybe it's the
>> general handling of macros which is at stake, but in any case it
>> shouldn't be unsolvable.
> 2011-01-07  Johan Bockgård  <bojohan@gnu.org>
> 	* emacs-lisp/unsafep.el (unsafep): Handle backquoted forms.

> diff --git a/lisp/emacs-lisp/unsafep.el b/lisp/emacs-lisp/unsafep.el
> index a62f8de..5dee2af 100644
> --- a/lisp/emacs-lisp/unsafep.el
> +++ b/lisp/emacs-lisp/unsafep.el
> @@ -202,6 +202,9 @@ UNSAFEP-VARS is a list of symbols with local bindings."
>  	      (dolist (x (nthcdr 3 form))
>  		(setq reason (unsafep-progn (cdr x)))
>  		(if reason (throw 'unsafep reason))))))
> +       ((eq fun '\`)
> +	;; Backquoted form - safe if its expansion is.
> +	(unsafep (cdr (backquote-process (cadr form)))))

Yes, that looks fine.  Feel free to install this patch, thanks.


        Stefan

Re: theme and a question about creating them

[Chong Yidong]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> So, I think it's OK to use unsafep to weed out the "safe" themes, and
> prompt users for the remaining themes.

This is now implemented.  It should now be possible to put arbitrary
elisp in theme files; if Emacs can't determine that the file is safe,
the user is queried.  The new variable `custom-safe-theme-files' stores
a list of known-safe themes.

Re: theme and a question about creating them

[Johan Bockgård]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Yes, that looks fine.  Feel free to install this patch, thanks.

Done.

Re: theme and a question about creating them

[Johan Bockgård]

Chong Yidong <cyd@stupidchicken.com> writes:

> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>> So, I think it's OK to use unsafep to weed out the "safe" themes, and
>> prompt users for the remaining themes.
>
> This is now implemented.  It should now be possible to put arbitrary
> elisp in theme files; if Emacs can't determine that the file is safe,
> the user is queried.

But custom-theme-set-variables and custom-theme-set-faces are actually
not safe functions.

Re: theme and a question about creating them

[Johan Bockgård]

Ping?

It doesn't make much sense to run the theme through `unsafep' while
marking `custom-theme-set-variables' and `custom-theme-set-faces' as
"safe functions" (which they aren't).


Johan Bockgård <bojohan@gnu.org> writes:

> Chong Yidong <cyd@stupidchicken.com> writes:
>
>> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>>
>>> So, I think it's OK to use unsafep to weed out the "safe" themes, and
>>> prompt users for the remaining themes.
>>
>> This is now implemented.  It should now be possible to put arbitrary
>> elisp in theme files; if Emacs can't determine that the file is safe,
>> the user is queried.
>
> But custom-theme-set-variables and custom-theme-set-faces are actually
> not safe functions.

Re: theme and a question about creating them

[Chong Yidong]

Johan Bockgård <bojohan@gnu.org> writes:

> It doesn't make much sense to run the theme through `unsafep' while
> marking `custom-theme-set-variables' and `custom-theme-set-faces' as
> "safe functions" (which they aren't).

A user who loads a theme would expects it to do *something*, so I'd
argue that those two functions should not be considered unsafe while
performing such an operation.

If this really is a concern, how bout marking only
custom-theme-set-faces as safe?  Then the user can be prompted for any
(non-built-in) theme that changes variables, but themes that only fiddle
with faces will be accepted.

Re: theme and a question about creating them

[Stefan Monnier]

>> It doesn't make much sense to run the theme through `unsafep' while
>> marking `custom-theme-set-variables' and `custom-theme-set-faces' as
>> "safe functions" (which they aren't).
> A user who loads a theme would expects it to do *something*, so I'd
> argue that those two functions should not be considered unsafe while
> performing such an operation.

But if it sets a hook to trigger a nuclear explosion, the user might
be in for a nasty surprise.

> If this really is a concern, how bout marking only
> custom-theme-set-faces as safe?  Then the user can be prompted for any
> (non-built-in) theme that changes variables, but themes that only
> fiddle with faces will be accepted.

I guess that one could be safe, tho IIRC we can set :height to
a function, so there's still a risk.


        Stefan