Too many (interactive) defuns

[wailo]

When I hit M-x, I get 91 commands from vterm, one for each keyboard action. Is there a reason why they are interactive? if not, I will be happy to submit a pr.

[Sbozzolo]

We need to bind those functions to the respective keys, so they must be interactive.

[wailo]

This is a workaround, I might have contaminated the code, don't hate me. The idea is to wrap the function in a lambda with an interactive flag, then it is not necessary to declare all key-press functions interactive, something like this: (define-key vterm-mode-map (kbd "M-<") (lambda () (interactive) (vterm--self-insert)))

With the code below, which I have tested briefly, there are only 9 vterm commands, which keeps my M-x menu sane.

Edit: Added a diff patch

index 9c73f18..01e17a7 100644
--- a/vterm.el
+++ b/vterm.el
@@ -524,7 +524,6 @@ Optional argument RESET clears all the errors."
              (doc-string 3))
     `(defun ,(intern (format "vterm-send-%s" key))()
        ,(format "Sends %s to the libvterm."  key)
-       (interactive)
        (vterm-send-key ,(char-to-string (get-byte (1- (length key)) key)) nil
                        ,(string-prefix-p "M-" key)
                        ,(string-prefix-p "C-" key))))
@@ -553,7 +552,7 @@ Exceptions are defined by `vterm-keymap-exceptions'."
                  collect key))
   (mapc (lambda (key)
           (define-key vterm-mode-map (kbd key)
-            (intern (format "vterm-send-%s" key))))
+            (lambda () (interactive) (intern (format "vterm-send-%s" key)))))
         (cl-loop for prefix in '("C-" "M-")
                  append (cl-loop for char from ?a to ?z
                                  for key = (format "%s%c" prefix char)
@@ -563,60 +562,60 @@ Exceptions are defined by `vterm-keymap-exceptions'."
 (vterm--exclude-keys vterm-keymap-exceptions)

 ;; Keybindings
-(define-key vterm-mode-map (kbd "M-<")                 #'vterm--self-insert)
-(define-key vterm-mode-map (kbd "M->")                 #'vterm--self-insert)
-(define-key vterm-mode-map [tab]                       #'vterm-send-tab)
-(define-key vterm-mode-map (kbd "TAB")                 #'vterm-send-tab)
-(define-key vterm-mode-map [backtab]                   #'vterm--self-insert)
-(define-key vterm-mode-map [backspace]                 #'vterm-send-backspace)
-(define-key vterm-mode-map (kbd "DEL")                 #'vterm-send-backspace)
-(define-key vterm-mode-map [delete]                    #'vterm-send-delete)
-(define-key vterm-mode-map [M-backspace]               #'vterm-send-meta-backspace)
-(define-key vterm-mode-map (kbd "M-DEL")               #'vterm-send-meta-backspace)
-(define-key vterm-mode-map [return]                    #'vterm-send-return)
-(define-key vterm-mode-map (kbd "RET")                 #'vterm-send-return)
-(define-key vterm-mode-map [left]                      #'vterm-send-left)
-(define-key vterm-mode-map [right]                     #'vterm-send-right)
-(define-key vterm-mode-map [up]                        #'vterm-send-up)
-(define-key vterm-mode-map [down]                      #'vterm-send-down)
-(define-key vterm-mode-map [prior]                     #'vterm-send-prior)
-(define-key vterm-mode-map [next]                      #'vterm-send-next)
-(define-key vterm-mode-map [home]                      #'vterm--self-insert)
-(define-key vterm-mode-map [end]                       #'vterm--self-insert)
-(define-key vterm-mode-map [escape]                    #'vterm--self-insert)
-(define-key vterm-mode-map [remap yank]                #'vterm-yank)
-(define-key vterm-mode-map [remap yank-pop]            #'vterm-yank-pop)
-(define-key vterm-mode-map [remap mouse-yank-primary]  #'vterm-yank-primary)
-(define-key vterm-mode-map (kbd "C-SPC")               #'vterm--self-insert)
-(define-key vterm-mode-map (kbd "S-SPC")               #'vterm-send-space)
-(define-key vterm-mode-map (kbd "C-_")                 #'vterm--self-insert)
-(define-key vterm-mode-map (kbd "C-/")                 #'vterm-undo)
-(define-key vterm-mode-map (kbd "M-.")                 #'vterm-send-meta-dot)
-(define-key vterm-mode-map (kbd "M-,")                 #'vterm-send-meta-comma)
-(define-key vterm-mode-map (kbd "C-c C-y")             #'vterm--self-insert)
-(define-key vterm-mode-map (kbd "C-c C-c")             #'vterm-send-C-c)
-(define-key vterm-mode-map (kbd "C-c C-l")             #'vterm-clear-scrollback)
-(define-key vterm-mode-map (kbd "C-l")                 #'vterm-clear)
-(define-key vterm-mode-map (kbd "C-\\")                #'vterm-send-ctrl-slash)
-(define-key vterm-mode-map (kbd "C-c C-g")             #'vterm-send-C-g)
-(define-key vterm-mode-map (kbd "C-c C-u")             #'vterm-send-C-u)
-(define-key vterm-mode-map [remap self-insert-command] #'vterm--self-insert)
-
-(define-key vterm-mode-map (kbd "C-c C-r")             #'vterm-reset-cursor-point)
-(define-key vterm-mode-map (kbd "C-c C-n")             #'vterm-next-prompt)
-(define-key vterm-mode-map (kbd "C-c C-p")             #'vterm-previous-prompt)
+(define-key vterm-mode-map (kbd "M-<")                 (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map (kbd "M->")                 (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map [tab]                       (lambda () (interactive) (vterm-send-tab)))
+(define-key vterm-mode-map (kbd "TAB")                 (lambda () (interactive) (vterm-send-tab)))
+(define-key vterm-mode-map [backtab]                   (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map [backspace]                 (lambda () (interactive) (vterm-send-backspace)))
+(define-key vterm-mode-map (kbd "DEL")                 (lambda () (interactive) (vterm-send-backspace)))
+(define-key vterm-mode-map [delete]                    (lambda () (interactive) (vterm-send-delete)))
+(define-key vterm-mode-map [M-backspace]               (lambda () (interactive) (vterm-send-meta-backspace)))
+(define-key vterm-mode-map (kbd "M-DEL")               (lambda () (interactive) (vterm-send-meta-backspace)))
+(define-key vterm-mode-map [return]                    (lambda () (interactive) (vterm-send-return)))
+(define-key vterm-mode-map (kbd "RET")                 (lambda () (interactive) (vterm-send-return)))
+(define-key vterm-mode-map [left]                      (lambda () (interactive) (vterm-send-left)))
+(define-key vterm-mode-map [right]                     (lambda () (interactive) (vterm-send-right)))
+(define-key vterm-mode-map [up]                        (lambda () (interactive) (vterm-send-up)))
+(define-key vterm-mode-map [down]                      (lambda () (interactive) (vterm-send-down)))
+(define-key vterm-mode-map [prior]                     (lambda () (interactive) (vterm-send-prior)))
+(define-key vterm-mode-map [next]                      (lambda () (interactive) (vterm-send-next)))
+(define-key vterm-mode-map [home]                      (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map [end]                       (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map [escape]                    (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map [remap yank]                (lambda (&optional arg) (interactive "P") (vterm-yank arg)))
+(define-key vterm-mode-map [remap yank-pop]            (lambda (&optional arg) (interactive "P") (vterm-yank-pop arg)))
+(define-key vterm-mode-map [remap mouse-yank-primary]  (lambda () (interactive) (vterm-yank-primary)))
+(define-key vterm-mode-map (kbd "C-SPC")               (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map (kbd "S-SPC")               (lambda () (interactive) (vterm-send-space)))
+(define-key vterm-mode-map (kbd "C-_")                 (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map (kbd "C-/")                 (lambda () (interactive) (vterm-undo)))
+(define-key vterm-mode-map (kbd "M-.")                 (lambda () (interactive) (vterm-send-meta-dot)))
+(define-key vterm-mode-map (kbd "M-,")                 (lambda () (interactive) (vterm-send-meta-comma)))
+(define-key vterm-mode-map (kbd "C-c C-y")             (lambda () (interactive) (vterm--self-insert)))
+(define-key vterm-mode-map (kbd "C-c C-c")             (lambda () (interactive) (vterm-send-C-c)))
+(define-key vterm-mode-map (kbd "C-c C-l")             (lambda () (interactive) (vterm-clear-scrollback)))
+(define-key vterm-mode-map (kbd "C-l")                 (lambda (&optional arg) (interactive "P") (vterm-clear arg)))
+(define-key vterm-mode-map (kbd "C-\\")                (lambda () (interactive) (vterm-send-ctrl-slash)))
+(define-key vterm-mode-map (kbd "C-c C-g")             (lambda () (interactive) (vterm-send-C-g)))
+(define-key vterm-mode-map (kbd "C-c C-u")             (lambda () (interactive) (vterm-send-C-u)))
+(define-key vterm-mode-map [remap self-insert-command] (lambda () (interactive) (vterm--self-insert)))
+
+(define-key vterm-mode-map (kbd "C-c C-r")             (lambda () (interactive) (vterm-reset-cursor-point)))
+(define-key vterm-mode-map (kbd "C-c C-n")             (lambda (n) (interactive "P") (vterm-next-prompt n)))
+(define-key vterm-mode-map (kbd "C-c C-p")             (lambda (n) (interactive "P") (vterm-previous-prompt n)))

 (define-key vterm-mode-map (kbd "C-c C-t")             #'vterm-copy-mode)

 (defvar vterm-copy-mode-map (make-sparse-keymap))
 (define-key vterm-copy-mode-map (kbd "C-c C-t")        #'vterm-copy-mode)
-(define-key vterm-copy-mode-map [return]               #'vterm-copy-mode-done)
-(define-key vterm-copy-mode-map (kbd "RET")            #'vterm-copy-mode-done)
-(define-key vterm-copy-mode-map (kbd "C-c C-r")        #'vterm-reset-cursor-point)
-(define-key vterm-copy-mode-map (kbd "C-a")            #'vterm-beginning-of-line)
-(define-key vterm-copy-mode-map (kbd "C-e")            #'vterm-end-of-line)
-(define-key vterm-copy-mode-map (kbd "C-c C-n")        #'vterm-next-prompt)
-(define-key vterm-copy-mode-map (kbd "C-c C-p")        #'vterm-previous-prompt)
+(define-key vterm-copy-mode-map [return]               (lambda (arg) (interactive "P") (vterm-copy-mode-done arg)))
+(define-key vterm-copy-mode-map (kbd "RET")            (lambda (arg) (interactive "P") (vterm-copy-mode-done arg)))
+(define-key vterm-copy-mode-map (kbd "C-c C-r")        (lambda () (interactive) (vterm-reset-cursor-point)))
+(define-key vterm-copy-mode-map (kbd "C-a")            (lambda () (interactive) (vterm-beginning-of-line)))
+(define-key vterm-copy-mode-map (kbd "C-e")            (lambda () (interactive) (vterm-end-of-line)))
+(define-key vterm-copy-mode-map (kbd "C-c C-n")        (lambda (n) (interactive "P") (vterm-next-prompt n)))
+(define-key vterm-copy-mode-map (kbd "C-c C-p")        (lambda (n) (interactive "P") (vterm-previous-prompt n)) )

 (define-minor-mode vterm-copy-mode
@@ -652,7 +651,6 @@ current line is killed from start to end.
 The option `vterm-copy-exclude-prompt' controls if the prompt
 should be included in a line copy.  Using the universal prefix ARG
 will invert `vterm-copy-exclude-prompt' for that call."
-  (interactive "P")
   (unless vterm-copy-mode
     (user-error "This command is effective only in vterm-copy-mode"))
   (unless (use-region-p)
@@ -669,7 +667,6 @@ will invert `vterm-copy-exclude-prompt' for that call."

 (defun vterm--self-insert ()
   "Send invoking key to libvterm."
-  (interactive)
   (when vterm--term
     (let* ((modifiers (event-modifiers last-input-event))
            (shift (memq 'shift modifiers))
@@ -700,17 +697,14 @@ will invert `vterm-copy-exclude-prompt' for that call."

 (defun vterm-send-start ()
   "Output from the system is started when the system receives START."
-  (interactive)
   (vterm-send-key "<start>"))

 (defun vterm-send-stop ()
   "Output from the system is stopped when the system receives STOP."
-  (interactive)
   (vterm-send-key "<stop>"))

 (defun vterm-send-return ()
   "Send `C-m' to the libvterm."
-  (interactive)
   (when vterm--term
     (if (vterm--get-icrnl vterm--term)
         (process-send-string vterm--process "\C-j")
@@ -718,72 +712,58 @@ will invert `vterm-copy-exclude-prompt' for that call."

 (defun vterm-send-tab ()
   "Send `<tab>' to the libvterm."
-  (interactive)
   (vterm-send-key "<tab>"))

 (defun vterm-send-space ()
   "Send `<space>' to the libvterm."
-  (interactive)
   (vterm-send-key " "))

 (defun vterm-send-backspace ()
   "Send `<backspace>' to the libvterm."
-  (interactive)
   (vterm-send-key "<backspace>"))

 (defun vterm-send-delete ()
   "Send `<delete>' to the libvterm."
-  (interactive)
   (vterm-send-key "<delete>"))

 (defun vterm-send-meta-backspace ()
   "Send `M-<backspace>' to the libvterm."
-  (interactive)
   (vterm-send-key "<backspace>" nil t))

 (defun vterm-send-up ()
   "Send `<up>' to the libvterm."
-  (interactive)
   (vterm-send-key "<up>"))

 (defun vterm-send-down ()
   "Send `<down>' to the libvterm."
-  (interactive)
   (vterm-send-key "<down>"))

 (defun vterm-send-left ()
   "Send `<left>' to the libvterm."
-  (interactive)
   (vterm-send-key "<left>"))

 (defun vterm-send-right ()
   "Send `<right>' to the libvterm."
-  (interactive)
   (vterm-send-key "<right>"))

 (defun vterm-send-prior ()
   "Send `<prior>' to the libvterm."
-  (interactive)
   (vterm-send-key "<prior>"))

 (defun vterm-send-next ()
   "Send `<next>' to the libvterm."
-  (interactive)
   (vterm-send-key "<next>"))

 (defun vterm-send-meta-dot ()
   "Send `M-.' to the libvterm."
-  (interactive)
   (vterm-send-key "." nil t))

 (defun vterm-send-meta-comma ()
   "Send `M-,' to the libvterm."
-  (interactive)
   (vterm-send-key "," nil t))

 (defun vterm-send-ctrl-slash ()
   "Send `C-\' to the libvterm."
-  (interactive)
   (vterm-send-key "\\" nil nil t))

 (defun vterm-send-escape ()
@@ -793,7 +773,6 @@ will invert `vterm-copy-exclude-prompt' for that call."

 (defun vterm-clear-scrollback ()
   "Send `<clear-scrollback>' to the libvterm."
-  (interactive)
   (vterm-send-key "<clear_scrollback>"))

 (defun vterm-clear (&optional arg)
@@ -804,7 +783,6 @@ will invert `vterm-copy-exclude-prompt' for that call."

 This behavior can be altered by calling `vterm-clear' with a
 prefix argument ARG or with \\[universal-argument]."
-  (interactive "P")
   (if (or
        (and vterm-clear-scrollback-when-clearing (not arg))
        (and arg (not vterm-clear-scrollback-when-clearing)))
@@ -813,14 +791,12 @@ prefix argument ARG or with \\[universal-argument]."

 (defun vterm-undo ()
   "Send `C-_' to the libvterm."
-  (interactive)
   (vterm-send-key "_" nil nil t))

 (defun vterm-yank (&optional arg)
   "Yank (paste) text in vterm.

 Argument ARG is passed to `yank'."
-  (interactive "P")
   (let ((inhibit-read-only t))
     (cl-letf (((symbol-function 'insert-for-yank)
                #'(lambda (str) (vterm-send-string str t))))
@@ -828,7 +804,6 @@ Argument ARG is passed to `yank'."

 (defun vterm-yank-primary ()
   "Yank text from the primary selection in vterm."
-  (interactive)
   (let ((inhibit-read-only t)
         (primary (gui-get-primary-selection)))
     (cl-letf (((symbol-function 'insert-for-yank)
@@ -839,7 +814,6 @@ Argument ARG is passed to `yank'."
   "Replaced text just yanked with the next entry in the kill ring.

 Argument ARG is passed to `yank'"
-  (interactive "p")
   (let ((inhibit-read-only t)
         (yank-undo-function #'(lambda (_start _end) (vterm-undo))))
     (cl-letf (((symbol-function 'insert-for-yank)
@@ -1114,7 +1088,6 @@ in README."

 (defun vterm-next-prompt (n)
   "Move to end of Nth next prompt in the buffer."
-  (interactive "p")
   (if (and vterm-use-vterm-prompt-detection-method
            (vterm--prompt-tracking-enabled-p))
       (let ((pt (point))
@@ -1127,7 +1100,6 @@ in README."

 (defun vterm-previous-prompt (n)
   "Move to end of Nth previous prompt in the buffer."
-  (interactive "p")
   (if (and vterm-use-vterm-prompt-detection-method
            (vterm--prompt-tracking-enabled-p))
       (let ((pt (point))
@@ -1190,7 +1162,6 @@ More information see `vterm--prompt-tracking-enabled-p' and
 Move the point to the first character after the shell prompt on this line.
 If the point is already there, move to the beginning of the line.
 Effectively toggle between the two positions."
-  (interactive)
   (if (vterm--at-prompt-p)
       (goto-char (vterm--get-beginning-of-line))
     (goto-char (max (or (vterm--get-prompt-point) 0)
@@ -1198,7 +1169,6 @@ Effectively toggle between the two positions."

 (defun vterm-end-of-line ()
   "Move point to the end of the line, bypassing line wraps."
-  (interactive)
   (goto-char (vterm--get-end-of-line)))

 (defun vterm-reset-cursor-point ()

[wailo]

Would you prefer a PR instead?

[Sbozzolo]

I am planning to clean-up how keys are assigned in vterm to make the code more maintainable, and I will deal with this while working on that. I believe that this issue by itself has a very low priority.

What you can do is to edit your previous message to include a patch as opposed to the entire file, as it is not easy to see what you did.

[wailo]

Done.

[tsdh]

@wailo, I don't see how this change would improve things. There is nothing inherently bad with having many commands. However, the proposed interactive lambda wrappers do have multiple clear downsides:

1. C-h k <some-key> will now just tell the key is bound to some interactive anonymous closure with no docstring.
2. C-h b (show all keybindings of the current minor modes and major mode) becomes useless.
3. (define-key vterm-mode-map [remap vterm-some-command] #'vterm-some-other-command) won't work if vterm-some-command isn't a named command.

I think I can come up with many more reasons why this is not a sensible change.

[wailo]

Sorry for the late response. This pull request is now too old and can be declined. But I would like to point out that having too many interactive commands slows down helm/ivy-M-x significantly. Most of the interactive commands here represent a single key press, which most likely will be invoked by the keyboard, not via the M-x menu.