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

FR: way to autoload non-interactive functions/macros #851

Closed aspiers closed 2 years ago

aspiers commented 4 years ago

I looked for a convenient equivalent to :commands which creates the autoloaded function(s) as non-interactive (so that they don't pollute M-x completion from counsel-M-x, amx, etc.) but couldn't find one. I know I can achieve this manually, e.g.

(use-package org-crypt
  :requires org
  :ensure nil
  :defer t
  :init
  (autoload 'org-crypt-use-before-save-magic "org-crypt"))

but it would be much nicer if I could write:

(use-package org-crypt
  :requires org
  :ensure nil
  :functions org-crypt-use-before-save-magic)
aspiers commented 4 years ago

Just realised this applies to macros too.

aspiers commented 4 years ago

... so maybe the keyword should be :autoloads rather than :functions.

conao3 commented 3 years ago

How about?

(use-package org-crypt
  :commands org-crypt-use-before-save-magic)

It should generate autoload statement that you want

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

(macroexpand-1
 '(use-package org-crypt
    :commands org-crypt-use-before-save-magic))
;;=> (unless (fboundp 'org-crypt-use-before-save-magic)
;;     (autoload #'org-crypt-use-before-save-magic "org-crypt" nil t))
;;   nil
aspiers commented 3 years ago

That's what I already tried, and as described above it created an interactive autoload. The whole point of this issue was to find an alternative which is non-interactive and doesn't pollute the list of interactive commands which can be completed after M-x. Having said that, it seems from your macroexpand and from checking my running emacs that maybe this problem was already fixed, because I don't see the issue any more. Very odd.

conao3 commented 3 years ago

Oh, sorry I misunderstood your issue. I showed use-pacakge generates interactive autoload so you do M-x, you should see the command (but the function is not interactive)

I think, there is no keyword that you want. Maybe PR to org-crypt to add ;;;###autoload comment if you want to autoload...?

aspiers commented 3 years ago

@conao3 commented on February 22, 2021 4:04 PM:

Oh, sorry I misunderstood your issue. I showed use-pacakge generates interactive autoload so you do M-x, you should see the command (but the function is not interactive)

Right, that is exactly the issue.

I think, there is no keyword that you want.

I'm not sure - look at the expansion you shared:

;; (autoload #'org-crypt-use-before-save-magic "org-crypt" nil t))

The arguments for autoload are:

(autoload FUNCTION FILE &optional DOCSTRING INTERACTIVE TYPE)

so apparently it is creating a non-interactive autoload here?

Maybe PR to org-crypt to add ;;;###autoload comment if you want to autoload...?

Oh yeah good point, I suppose individual packages should be responsible for creating correct autoloads.

conao3 commented 3 years ago

No, we define org-crypt-use-before-save-magic as a interctive one.

(autoload #'org-crypt-use-before-save-magic "org-crypt"    nil       t)
(autoload FUNCTION                          FILE &optional DOCSTRING INTERACTIVE TYPE)

Here is a good news, org-crypt-use-before-save-magic is autoloaded in org HEAD. please install ratest org, this issue should be resolved.

https://code.orgmode.org/bzg/org-mode/src/master/lisp/org-crypt.el#L307-L312

;;;###autoload
(defun org-crypt-use-before-save-magic ()
  "Add a hook to automatically encrypt entries before a file is saved to disk."
  (add-hook
   'org-mode-hook
   (lambda () (add-hook 'before-save-hook 'org-encrypt-entries nil t))))
aspiers commented 3 years ago

@conao3 commented on February 22, 2021 4:38 PM:

No, we define org-crypt-use-before-save-magic as a interctive one.

(autoload #'org-crypt-use-before-save-magic "org-crypt"    nil       t)
(autoload FUNCTION                          FILE &optional DOCSTRING INTERACTIVE TYPE)

Whoops, I can't count :laughing: No wonder I was getting confused.

Here is a good news, org-crypt-use-before-save-magic is autoloaded in org HEAD. please install ratest org, this issue should be resolved.

https://code.orgmode.org/bzg/org-mode/src/master/lisp/org-crypt.el#L307-L312

;;;###autoload
(defun org-crypt-use-before-save-magic ()
  "Add a hook to automatically encrypt entries before a file is saved to disk."
  (add-hook
   'org-mode-hook
   (lambda () (add-hook 'before-save-hook 'org-encrypt-entries nil t))))

Great, thanks!

Do you think this is a valid feature request for other cases, or should packages always define the autoloads themselves?

conao3 commented 3 years ago

Do you think this is a valid feature request for other cases, or should packages always define the autoloads themselves?

This is a valid feature request for other cases, but I think, PR ;;;###autoload comment should help other users too, I recommend this way. or, we should define :commands* to generate no-interactive one.

conao3 commented 3 years ago

With this patch, you can use commands* keyword.

add-commands-keyword 3732a3ebaaf7e59d0228be698b385d5a3c57ac4b
Author:     Naoya Yamashita <conao3@gmail.com>
AuthorDate: Tue Feb 23 02:49:18 2021 +0900
Commit:     Naoya Yamashita <conao3@gmail.com>
CommitDate: Tue Feb 23 03:02:53 2021 +0900

Parent:     a7422fb Merge pull request #910 from minad/improved-unbind
Merged:     add-commands-keyword master
Contained:  add-commands-keyword
Follows:    2.4 (65)

add commands* keyword

commands* is similar to :command but this generate autoload
statement as *no-interactive* function.

1 file changed, 26 insertions(+), 1 deletion(-)
use-package-core.el | 27 ++++++++++++++++++++++++++-

modified   use-package-core.el
@@ -94,6 +94,7 @@
     ;; Any other keyword that also declares commands to be autoloaded (such as
     ;; :bind) must appear before this keyword.
     :commands
+    :commands*
     :init
     :defer
     :demand
@@ -118,7 +119,8 @@ declaration is incorrect."
 (defcustom use-package-deferring-keywords
   '(:bind-keymap
     :bind-keymap*
-    :commands)
+    :commands
+    :commands*)
   "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
@@ -1309,6 +1311,28 @@ meaning:
       (delete-dups arg)))
    (use-package-process-keywords name rest state)))

+;;;; :commands
+
+(defalias 'use-package-normalize/:commands* 'use-package-normalize/:commands)
+
+(defun use-package-handler/:commands* (name _keyword arg rest state)
+  (use-package-concat
+   ;; Since we deferring load, establish any necessary autoloads, and also
+   ;; keep the byte-compiler happy.
+   (let ((name-string (use-package-as-string name)))
+     (cl-mapcan
+      #'(lambda (command)
+          (when (symbolp command)
+            (append
+             (unless (plist-get state :demand)
+               `((unless (fboundp ',command)
+                   (autoload #',command ,name-string))))
+             (when (bound-and-true-p byte-compile-current-file)
+               `((eval-when-compile
+                   (declare-function ,command ,name-string)))))))
+      (delete-dups arg)))
+   (use-package-process-keywords name rest state)))
+
 ;;;; :defer

 (defalias 'use-package-normalize/:defer 'use-package-normalize-predicate)
@@ -1570,6 +1594,7 @@ this file.  Usage:
                  package.  This is useful if the package is being lazily
                  loaded, and you wish to conditionally call functions in your
                  `:init' block that are defined in the package.
+:commands*       Similar to :commands, but it for no-interactive one.
 :hook            Specify hook(s) to attach this package to.

 :bind            Bind keys, and define autoloads for the bound commands.
(macroexpand-1
 '(use-package org-crypt
    :commands* org-crypt-use-before-save-magic))
;;=> (unless (fboundp 'org-crypt-use-before-save-magic)
;;     (autoload #'org-crypt-use-before-save-magic "org-crypt"))
aspiers commented 3 years ago

Nice! Thanks a lot!

aspiers commented 2 years ago

917 was merged, so this is now resolved. Thanks a lot @conao3 and @jwiegley!