|
From: | Basil L. Contovounesios |
Subject: | Re: Byte-compilation of custom themes |
Date: | Fri, 11 May 2018 16:16:09 +0100 |
User-agent: | Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) |
Eli Zaretskii <address@hidden> writes: >> From: "Basil L. Contovounesios" <address@hidden> >> Date: Thu, 10 May 2018 03:49:35 +0100 >> Cc: address@hidden >> >> 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. The function custom-theme--load-path already incorporates the file-directory-p check, so it is technically redundant here. Would you rather it be kept regardless? >> - (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? Though I personally prefer to consistently #'-quote function symbols in my own code, both for the extra byte-compiler check and narrower in-buffer completion, I have no strong opinion here; I was simply making the change in a sweeping fashion, in line with what I had perceived as a welcome style. Out of curiosity, though, what makes key bindings special w.r.t. quoting? >> 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? Because it hadn't occurred to silly us that we might want to additionally protect against evaluation errors. I reattach the patches, updated for the last two points of feedback and to remove the duplicate 'Custom Themes' heading. Thanks, -- Basil
0001-Improve-loading-of-byte-compiled-custom-themes.patch
Description: Text Data
0002-Disable-no-byte-compile-in-built-in-themes.patch
Description: Text Data
0003-Fix-custom-available-themes-file-expansion.patch
Description: Text Data
0004-lisp-custom.el-Use-lexical-binding.patch
Description: Text Data
0005-lisp-cus-theme.el-Use-lexical-binding.patch
Description: Text Data
0006-Minor-custom.el-simplifications.patch
Description: Text Data
0007-Minor-cus-theme.el-simplifications.patch
Description: Text Data
0008-Minor-subr-x-tweaks.patch
Description: Text Data
[Prev in Thread] | Current Thread | [Next in Thread] |