From 8fefa49d39dde99dfd13ff9551723ec387a3bfba Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Thu, 7 Dec 2017 13:14:32 -0800 Subject: [PATCH] Changes to the way auto-deferral is indicated This change adds a new extension hook `use-package-autoloads/` for specifying exactly which autoloads a keyword should imply. This is the proper way to indicate autoloads, rather than adding to the `:commands` entry as was done before. Further, autoloading now must occur in order to cause implied deferred loading; if :bind is used with only lambda forms, for example, this will not cause deferred loading without `:defer t`. --- etc/USE-PACKAGE-NEWS | 26 +++- lisp/use-package/use-package-bind-key.el | 25 ++-- lisp/use-package/use-package-core.el | 160 ++++++++++++--------- test/lisp/use-package/use-package-tests.el | 79 ++++++---- 4 files changed, 181 insertions(+), 109 deletions(-) diff --git a/etc/USE-PACKAGE-NEWS b/etc/USE-PACKAGE-NEWS index 76556be9b2e..c34376d3ffe 100644 --- a/etc/USE-PACKAGE-NEWS +++ b/etc/USE-PACKAGE-NEWS @@ -30,10 +30,6 @@ parameter, but are now done as an extension to `rest`. Please see `use-package-handler/:bind` for a canonical example. -- For extension authors, if you add a keyword to `use-package-keywords` whose - presence should indicate deferred loading, please also add it to - `use-package-deferring-keywords`. - ### Other changes - Upgrade license to GPL 3. @@ -154,6 +150,28 @@ it is loaded, `helm-descbinds` itself is not loaded until the user presses `C-h b`. +- For extension authors, if you add a keyword to `use-package-keywords` whose + presence should indicate deferred loading, please also add it to + `use-package-deferring-keywords`. Note that this is a bit of a sledgehammer, + in that the mere presence of these keywords implies deferred loading. For a + more subtle approach, see the new `use-package-autoloads/` support + mentioned in the next bullet. + +- For extension authors, if you wish deferred loading to possibly occur, + create functions named `use-package-autoloads/` for each keyword + that you define, returning an alist of the form `(SYMBOL . TYPE)` of symbols + to be autoloaded. `SYMBOL` should be an interactive function, and `TYPE` the + smybol `command`, but this functionality may be extended in future. These + autoloads are established if deferred loading is to happen. + +- If you specify a lambda form rather than a function symbol in any of the + constructs that *might* introduce autoloads: `:bind`, `:bind*`, + `:interpreter`, `:mode`, `:magic`, `:magic-fallback`, and `:hook`: then + deferred loading will no longer be implied, since there's nothing to + associate an autoload with that could later load the module. In these cases, + it will be as if you'd specified `:demand t`, in order to ensure the lambda + form is able to execute in the context of the loaded package. + - For extension authors, there is a new customization variable `use-package-merge-key-alist` that specifies how values passed to multiple occurences of the same key should be merged into a single value, during diff --git a/lisp/use-package/use-package-bind-key.el b/lisp/use-package/use-package-bind-key.el index 4a3d421522b..6a8e3fbe8eb 100644 --- a/lisp/use-package/use-package-bind-key.el +++ b/lisp/use-package/use-package-bind-key.el @@ -114,20 +114,23 @@ deferred until the prefix key sequence is pressed." ;;;###autoload (defalias 'use-package-normalize/:bind* 'use-package-normalize-binder) +;; jww (2017-12-07): This is too simplistic. It will fail to determine +;; autoloads in this situation: +;; (use-package foo +;; :bind (:map foo-map (("C-a" . func)))) +;;;###autoload +(defalias 'use-package-autoloads/:bind 'use-package-autoloads-mode) +;;;###autoload +(defalias 'use-package-autoloads/:bind* 'use-package-autoloads-mode) + ;;;###autoload (defun use-package-handler/:bind (name keyword args rest state &optional bind-macro) - (let* ((result (use-package-normalize-commands args)) - (nargs (car result)) - (commands (cdr result))) - (use-package-concat - (use-package-process-keywords name - (use-package-sort-keywords - (use-package-plist-append rest :commands commands)) - state) - `((ignore - (,(if bind-macro bind-macro 'bind-keys) - :package ,name ,@nargs)))))) + (use-package-concat + (use-package-process-keywords name rest state) + `((ignore + (,(if bind-macro bind-macro 'bind-keys) + :package ,name ,@(use-package-normalize-commands args)))))) (defun use-package-handler/:bind* (name keyword arg rest state) (use-package-handler/:bind name keyword arg rest state 'bind-keys*)) diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el index 175e023a5a4..001ffde1534 100644 --- a/lisp/use-package/use-package-core.el +++ b/lisp/use-package/use-package-core.el @@ -100,17 +100,15 @@ declaration is incorrect." :group 'use-package) (defcustom use-package-deferring-keywords - '(:bind - :bind* - :bind-keymap + '(:bind-keymap :bind-keymap* - :interpreter - :mode - :magic - :magic-fallback - :hook :commands) - "Unless `:demand' is used, keywords in this list imply deferred loading." + "Unless `:demand' is used, keywords in this list imply deferred loading. +The reason keywords like `:hook' are not in this list is that +they only imply deferred loading if they reference actual +function symbols that can be autoloaded from the module; whereas +the default keywords provided here always defer loading unless +otherwise requested." :type '(repeat symbol) :group 'use-package) @@ -160,7 +158,7 @@ See also `use-package-defaults', which uses this value." (and use-package-always-demand (not (plist-member args :defer)) (not (plist-member args :demand)))))) - "Alist of default values for `use-package' keywords. + "Default values for specified `use-package' keywords. Each entry in the alist is a list of three elements. The first element is the `use-package' keyword and the second is a form that can be evaluated to get the default value. The third element @@ -497,8 +495,9 @@ extending any keys already present." (xs (use-package-split-list #'keywordp (cdr input))) (args (car xs)) (tail (cdr xs)) - (normalizer (intern (concat "use-package-normalize/" - (symbol-name keyword)))) + (normalizer + (intern-soft (concat "use-package-normalize/" + (symbol-name keyword)))) (arg (and (functionp normalizer) (funcall normalizer name keyword args)))) (if (memq keyword use-package-keywords) @@ -562,6 +561,32 @@ extending any keys already present." (setq args (use-package-plist-maybe-put args (nth 0 spec) (eval (nth 1 spec)))))) + ;; Determine any autoloads implied by the keywords used. + (let ((iargs args) + commands) + (while iargs + (when (keywordp (car iargs)) + (let ((autoloads + (intern-soft (concat "use-package-autoloads/" + (symbol-name (car iargs)))))) + (when (functionp autoloads) + (setq commands + ;; jww (2017-12-07): Right now we just ignored the type of + ;; the autoload being requested, and assume they are all + ;; `command'. + (append (mapcar + #'car + (funcall autoloads name-symbol (car iargs) + (cadr iargs))) + commands))))) + (setq iargs (cddr iargs))) + (when commands + (setq args + ;; Like `use-package-plist-append', but removing duplicates. + (plist-put args :commands + (delete-dups + (append commands (plist-get args :commands))))))) + ;; If byte-compiling, pre-load the package so all its symbols are in ;; scope. This is done by prepending statements to the :preface. (when (bound-and-true-p byte-compile-current-file) @@ -593,10 +618,13 @@ extending any keys already present." use-package-deferring-keywords))) (setq args (append args '(:defer t)))) + ;; The :load keyword overrides :no-require (when (and (plist-member args :load) (plist-member args :no-require)) (setq args (use-package-plist-delete args :no-require))) + ;; If at this point no :load, :defer or :no-require has been seen, then + ;; :load the package itself. (when (and (not (plist-member args :load)) (not (plist-member args :defer)) (not (plist-member args :no-require))) @@ -851,22 +879,12 @@ If RECURSED is non-nil, recurse into sublists." (t v))) (defun use-package-normalize-commands (args) - "Map over ARGS of the form ((_ . F) ...). -Normalizing functional F's and returning a list of F's -representing symbols (that may need to be autloaded)." - (let ((nargs (mapcar - #'(lambda (x) - (if (consp x) - (cons (car x) - (use-package-normalize-function (cdr x))) - x)) args))) - (cons nargs - (delete - nil (mapcar - #'(lambda (x) - (and (consp x) - (use-package-non-nil-symbolp (cdr x)) - (cdr x))) nargs))))) + "Map over ARGS of the form ((_ . F) ...), normalizing functional F's." + (mapcar #'(lambda (x) + (if (consp x) + (cons (car x) (use-package-normalize-function (cdr x))) + x)) + args)) (defun use-package-normalize-mode (name keyword args) "Normalize arguments for keywords which add regexp/mode pairs to an alist." @@ -876,6 +894,27 @@ representing symbols (that may need to be autloaded)." #'use-package-recognize-function name))) +(defun use-package-autoloads-mode (name keyword args) + (mapcar + #'(lambda (x) (cons (cdr x) 'command)) + (cl-remove-if-not #'(lambda (x) + (and (consp x) + (use-package-non-nil-symbolp (cdr x)))) + args))) + +(defun use-package-handle-mode (name alist args rest state) + "Handle keywords which add regexp/mode pairs to an alist." + (use-package-concat + (use-package-process-keywords name rest state) + `((ignore + ,@(mapcar + #'(lambda (thing) + `(add-to-list + ',alist + ',(cons (use-package-normalize-regex (car thing)) + (cdr thing)))) + (use-package-normalize-commands args)))))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; ;;; Statistics @@ -1078,26 +1117,8 @@ meaning: ;;;; :interpreter -(defun use-package-handle-mode (name alist args rest state) - "Handle keywords which add regexp/mode pairs to an alist." - (let* ((result (use-package-normalize-commands args)) - (nargs (car result)) - (commands (cdr result))) - (use-package-concat - (use-package-process-keywords name - (use-package-sort-keywords - (use-package-plist-append rest :commands commands)) - state) - `((ignore - ,@(mapcar - #'(lambda (thing) - `(add-to-list - ',alist - ',(cons (use-package-normalize-regex (car thing)) - (cdr thing)))) - nargs)))))) - (defalias 'use-package-normalize/:interpreter 'use-package-normalize-mode) +(defalias 'use-package-autoloads/:interpreter 'use-package-autoloads-mode) (defun use-package-handler/:interpreter (name keyword arg rest state) (use-package-handle-mode name 'interpreter-mode-alist arg rest state)) @@ -1105,6 +1126,7 @@ meaning: ;;;; :mode (defalias 'use-package-normalize/:mode 'use-package-normalize-mode) +(defalias 'use-package-autoloads/:mode 'use-package-autoloads-mode) (defun use-package-handler/:mode (name keyword arg rest state) (use-package-handle-mode name 'auto-mode-alist arg rest state)) @@ -1112,6 +1134,7 @@ meaning: ;;;; :magic (defalias 'use-package-normalize/:magic 'use-package-normalize-mode) +(defalias 'use-package-autoloads/:magic 'use-package-autoloads-mode) (defun use-package-handler/:magic (name keyword arg rest state) (use-package-handle-mode name 'magic-mode-alist arg rest state)) @@ -1119,6 +1142,7 @@ meaning: ;;;; :magic-fallback (defalias 'use-package-normalize/:magic-fallback 'use-package-normalize-mode) +(defalias 'use-package-autoloads/:magic-fallback 'use-package-autoloads-mode) (defun use-package-handler/:magic-fallback (name keyword arg rest state) (use-package-handle-mode name 'magic-fallback-mode-alist arg rest state)) @@ -1145,31 +1169,27 @@ meaning: #'use-package-recognize-function name label arg)))) +(defalias 'use-package-autoloads/:hook 'use-package-autoloads-mode) + (defun use-package-handler/:hook (name keyword args rest state) "Generate use-package custom keyword code." - (let* ((result (use-package-normalize-commands args)) - (nargs (car result)) - (commands (cdr result))) - (use-package-concat - (use-package-process-keywords name - (use-package-sort-keywords - (use-package-plist-append rest :commands commands)) - state) - `((ignore - ,@(cl-mapcan - #'(lambda (def) - (let ((syms (car def)) - (fun (cdr def))) - (when fun - (mapcar - #'(lambda (sym) - `(add-hook - (quote ,(intern - (concat (symbol-name sym) - use-package-hook-name-suffix))) - (function ,fun))) - (if (use-package-non-nil-symbolp syms) (list syms) syms))))) - nargs)))))) + (use-package-concat + (use-package-process-keywords name rest state) + `((ignore + ,@(cl-mapcan + #'(lambda (def) + (let ((syms (car def)) + (fun (cdr def))) + (when fun + (mapcar + #'(lambda (sym) + `(add-hook + (quote ,(intern + (concat (symbol-name sym) + use-package-hook-name-suffix))) + (function ,fun))) + (if (use-package-non-nil-symbolp syms) (list syms) syms))))) + (use-package-normalize-commands args)))))) ;;;; :commands diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el index ffff7ced326..08ac5293a99 100644 --- a/test/lisp/use-package/use-package-tests.el +++ b/test/lisp/use-package/use-package-tests.el @@ -1001,30 +1001,30 @@ (ert-deftest use-package-test/:hook-1 () (let ((byte-compile-current-file t)) - (should - (equal - (expand-minimally - (use-package foo - :bind (("C-a" . key)) - :hook (hook . fun))) - '(progn - (eval-and-compile - (eval-when-compile - (with-demoted-errors - "Cannot load foo: %S" nil - (load "foo" nil t)))) - (unless (fboundp 'fun) - (autoload #'fun "foo" nil t)) - (eval-when-compile - (declare-function fun "foo")) - (unless (fboundp 'key) - (autoload #'key "foo" nil t)) - (eval-when-compile - (declare-function key "foo")) - (ignore - (add-hook 'hook-hook #'fun)) - (ignore - (bind-keys :package foo ("C-a" . key)))))))) + (match-expansion + (use-package foo + :bind (("C-a" . key)) + :hook (hook . fun)) + `(progn + (eval-and-compile + (eval-when-compile + (with-demoted-errors + "Cannot load foo: %S" nil + (load "foo" nil t)))) + (unless + (fboundp 'key) + (autoload #'key "foo" nil t)) + (eval-when-compile + (declare-function key "foo")) + (unless + (fboundp 'fun) + (autoload #'fun "foo" nil t)) + (eval-when-compile + (declare-function fun "foo")) + (ignore + (add-hook 'hook-hook #'fun)) + (ignore + (bind-keys :package foo ("C-a" . key))))))) (ert-deftest use-package-test/:hook-2 () (match-expansion @@ -1079,6 +1079,37 @@ #'(lambda nil (bind-key "" erefactor-map emacs-lisp-mode-map))))))))) +(ert-deftest use-package-test/:hook-6 () + (match-expansion + (use-package erefactor + :load-path "foo" + :after elisp-mode + :hook (emacs-lisp-mode . function)) + `(progn + (eval-and-compile + (add-to-list 'load-path "/Users/johnw/.emacs.d/foo")) + (eval-after-load 'elisp-mode + '(progn + (unless (fboundp 'function) + (autoload #'function "erefactor" nil t)) + (ignore + (add-hook 'emacs-lisp-mode-hook #'function))))))) + +(ert-deftest use-package-test/:hook-7 () + (match-expansion + (use-package erefactor + :load-path "foo" + :after elisp-mode + :hook (emacs-lisp-mode . (lambda () (function)))) + `(progn + (eval-and-compile + (add-to-list 'load-path "/Users/johnw/.emacs.d/foo")) + (eval-after-load 'elisp-mode + '(progn + (require 'erefactor nil nil) + (ignore + (add-hook 'emacs-lisp-mode-hook #'(lambda nil (function))))))))) + (ert-deftest use-package-test-normalize/:custom () (flet ((norm (&rest args) (apply #'use-package-normalize/:custom -- 2.39.2