Re: Byte-compilation of custom themes

[Eli Zaretskii <eliz@gnu.org>]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Thu, 10 May 2018 03:49:35 +0100
> Cc: emacs-devel@gnu.org
> 
> diff --git a/lisp/custom.el b/lisp/custom.el
> index 1fed5dce53..b286f49ff9 100644
> --- a/lisp/custom.el
> +++ b/lisp/custom.el
> @@ -1299,17 +1299,15 @@ custom-available-themes
>  loaded, and no effort is made to check that the files contain
>  valid Custom themes.  For a list of loaded themes, check the
>  variable `custom-known-themes'."
> -  (let (sym themes)
> +  (let ((suffix "-theme\\.el\\'")
> +        themes)
>      (dolist (dir (custom-theme--load-path))
> -      (when (file-directory-p dir)
> -	(dolist (file (file-expand-wildcards
> -		       (expand-file-name "*-theme.el" dir) t))
> -	  (setq file (file-name-nondirectory file))
> -	  (and (string-match "\\`\\(.+\\)-theme.el\\'" file)
> -	       (setq sym (intern (match-string 1 file)))
> -	       (custom-theme-name-valid-p sym)
> -	       (push sym themes)))))
> -    (nreverse (delete-dups themes))))
> +      (dolist (file (directory-files dir nil suffix))

The original code carefully verified that the members in
custom-theme--load-path are directories, whereas your proposal calls
directory-files on each member unconditionally, which will barf if a
file is not a directory.

> -    (define-key map "\C-x\C-s" 'custom-theme-write)
> -    (define-key map "q" 'Custom-buffer-done)
> -    (define-key map "n" 'widget-forward)
> -    (define-key map "p" 'widget-backward)
> +    (define-key map "\C-x\C-s" #'custom-theme-write)
> +    (define-key map "q" #'Custom-buffer-done)
> +    (define-key map "n" #'widget-forward)
> +    (define-key map "p" #'widget-backward)

Really?  Are we going to switch to #'foo even in key bindings?

> diff --git a/lisp/custom.el b/lisp/custom.el
> index b8004cfd74..ae917c0292 100644
> --- a/lisp/custom.el
> +++ b/lisp/custom.el
> @@ -633,14 +633,10 @@ custom-load-symbol
>      (let ((custom-load-recursion t))
>        ;; Load these files if not already done,
>        ;; to make sure we know all the dependencies of SYMBOL.
> -      (condition-case nil
> -	  (require 'cus-load)
> -	(error nil))
> -      (condition-case nil
> -	  (require 'cus-start)
> -	(error nil))
> +      (require 'cus-load nil t)
> +      (require 'cus-start nil t)
>        (dolist (load (get symbol 'custom-loads))
> -	(cond ((symbolp load) (condition-case nil (require load) (error nil)))
> +        (cond ((symbolp load) (require load nil t))
>  	      ;; This is subsumed by the test below, but it's much faster.
>  	      ((assoc load load-history))
>  	      ;; This was just (assoc (locate-library load) load-history)
> @@ -658,7 +654,7 @@ custom-load-symbol
>  	      ;; We are still loading it when we call this,
>  	      ;; and it is not in load-history yet.
>  	      ((equal load "cus-edit"))
> -	      (t (condition-case nil (load load) (error nil))))))))
> +              (t (load load t)))))))

Why are we dropping the safety nets here?

Thanks.