search

jwiegley / use-package

A use-package declaration for simplifying your .emacs
https://jwiegley.github.io/use-package
GNU General Public License v3.0
4.42k stars 260 forks source link

:hook lambda expansion #895

Closed icmor closed 3 years ago

icmor commented 3 years ago

I'm no expert by any means but I noticed that a use-package macro will load its package when setting a lambda expression as a hook. This may cause users to load packages while believing they are being deferred. Although, if the use-package expression binds any keys with :bind, the macro behaves as one would expect.

(use-package foo
  :hook
  (org-mode . (lambda () (message "Hello"))))

;; pp-macroexpand-last-sexp

(progn
  (use-package-ensure-elpa 'foo
               '(t)
               'nil)
  (defvar use-package--warning38
    #'(lambda
    (keyword err)
    (let
        ((msg
          (format "%s/%s: %s" 'foo keyword
              (error-message-string err))))
      (display-warning 'use-package msg :error))))
  (condition-case-unless-debug err
      (progn
    (if
        (not
         (require 'foo nil t))
        (display-warning 'use-package
                 (format "Cannot load %s" 'foo)
                 :error))
    (add-hook 'org-mode-hook
          #'(lambda nil
              (message "Hello"))))
    (error
     (funcall use-package--warning38 :catch err))))

Second case

(use-package foo
  :bind
  ("C-c f" . func)
  :hook
  (org-mode . (lambda () (message "Hello"))))

;; pp-macroexpand-last-sexp

(progn
  (use-package-ensure-elpa 'foo
               '(t)
               'nil)
  (defvar use-package--warning73
    #'(lambda
    (keyword err)
    (let
        ((msg
          (format "%s/%s: %s" 'foo keyword
              (error-message-string err))))
      (display-warning 'use-package msg :error))))
  (condition-case-unless-debug err
      (progn
    (unless
        (fboundp 'func)
      (autoload #'func "foo" nil t))
    (add-hook 'org-mode-hook
          #'(lambda nil
              (message "Hello")))
    (bind-keys :package foo
           ("C-c f" . func)))
    (error
     (funcall use-package--warning73 :catch err))))

This happens regardless of the ordering of :bind and :hook

A real example:

(use-package company-anaconda
  :hook
  (python-mode .
           (lambda () (add-to-list 'company-backends
                       '(company-anaconda :with
                              company-capf)))))

;; pp-macroexpand-last-sexp

(progn
  (use-package-ensure-elpa 'company-anaconda
               '(t)
               'nil)
  (defvar use-package--warning77
    #'(lambda
    (keyword err)
    (let
        ((msg
          (format "%s/%s: %s" 'company-anaconda keyword
              (error-message-string err))))
      (display-warning 'use-package msg :error))))
  (condition-case-unless-debug err
      (progn
    (if
        (not
         (require 'company-anaconda nil t))
        (display-warning 'use-package
                 (format "Cannot load %s" 'company-anaconda)
                 :error))
    (add-hook 'python-mode-hook
          #'(lambda nil
              (add-to-list 'company-backends
                   '(company-anaconda :with company-capf)))))
    (error
     (funcall use-package--warning77 :catch err))))
(use-package company-anaconda
  :bind
  ("C-c p" . run-python)
  :hook
  (python-mode .
           (lambda () (add-to-list 'company-backends
                       '(company-anaconda :with
                              company-capf)))))

;; pp-macroexpand-last-sexp

(progn
  (use-package-ensure-elpa 'company-anaconda
               '(t)
               'nil)
  (defvar use-package--warning80
    #'(lambda
    (keyword err)
    (let
        ((msg
          (format "%s/%s: %s" 'company-anaconda keyword
              (error-message-string err))))
      (display-warning 'use-package msg :error))))
  (condition-case-unless-debug err
      (progn
    (unless
        (fboundp 'run-python)
      (autoload #'run-python "company-anaconda" nil t))
    (add-hook 'python-mode-hook
          #'(lambda nil
              (add-to-list 'company-backends
                   '(company-anaconda :with company-capf))))
    (bind-keys :package company-anaconda
           ("C-c p" . run-python)))
    (error
     (funcall use-package--warning80 :catch err))))

PS: thanks for such a great package! Hope this isn't too hairy a problem.

conao3 commented 3 years ago

From use-package source. https://github.com/jwiegley/use-package/blob/a7422fb8ab1baee19adb2717b5b47b9c3812a84c/use-package-core.el#L118-L129

So, I think use-package cannot find your lambda expression as a autoload function (it is true), generate require statement. If you defer load force, please use :defer t keyword.

(setq use-package-expand-minimally t)
;;=> t

(ppp-macroexpand
 (use-package foo
   :hook
   (org-mode . (lambda () (message "Hello")))))
;;=> (progn
;;     (require 'foo nil nil)
;;     (add-hook 'org-mode-hook
;;               #'(lambda nil
;;                   (message "Hello"))))

(ppp-macroexpand
 (use-package foo
   :defer t
   :hook
   (org-mode . (lambda () (message "Hello")))))
;;=> (add-hook 'org-mode-hook
;;             #'(lambda nil
;;                 (message "Hello")))

(expand use-package is the fastest way to understand use-package as you do! Please check use-package-expand-minimally and ppp (and scratch-comment) to print more readable result)

Zalewa commented 1 year ago

@conao3, thanks for explaining that :hook implies :defer t only conditionally, but I believe this issue shouldn't have been closed without an action, @icmor.

The README.md still says at least 3 times that :hook implies :defer t without mentioning that this is conditional. I just fell into this trap. I added a use-package to my init.el with a :hook using a (lambda), thoroughly expecting that it will imply :defer t and won't load the package until the package is needed (the package is doc-view). Imagine my surprise when the package appeared in the load benchmark. I made it disappear again by adding an explicit :defer t. I began to suspect that this is a bug in use-package, but apparently (as explained in use-package-deferring-keywords`) this is an expected behavior, while the README states otherwise.

I think this is a problem with the README, because the README hammers it in that :hook implies :defer t, while this is untrue. The fact that there is some explanation in a docstring for use-package-deferring-keywords is not a sufficient warning when the frontend documentation omits this info and states something different instead.

This implies :defer t ((featurep 'doc-view) => nil):

(use-package doc-view
  :init
  (defun nolinum ()
      (linum-mode -1))
  :hook (doc-view-mode . nolinum))

And this doesn't ((featurep 'doc-view) => t):

(use-package doc-view
  :hook (doc-view-mode . (lambda ()
                            (linum-mode -1))))

Sorry, but with the way the README is currently written, this simply looks like a bug.