Tramp-login-shells custom doesn't match type

Thaodan commented 10 months ago

While customizing tramp-login-shells I have noticed that the set custom type doesn't match the format of the variable used in vterm.

For example this is the warning my Emacs showed me:

⛔ Warning (emacs): Value ‘(("docker" "/bin/sh") ("ssh" "/bin/zsh"))’ does not match type (alist :key-type string :value-type string)

My configuration:

(use-package vterm
  :bind (:map vterm-mode-map
              ("C-s" . isearch-forward))
  :config
  (setq vterm-max-scrollback 100000)
  ;; Include the title in vterm and multi-vterm buffers
  ;; setting multi-vterm-buffer-name isn't enough.
  (setq vterm-buffer-name-string "*vterm %s*")
  (setopt vterm-tramp-shells '(("docker" "/bin/sh")
                               ("ssh" "/bin/zsh"))))

Alf0nso commented 4 months ago

@Thaodan I Think this is can be fixed if you change setopt to setq instead:

(use-package vterm
  :ensure t
  :config
  (setq vterm-tramp-shells '(("docker" "/bin/sh")
                 ("ssh" "/bin/bash"))))

At least for me this worked...

Thaodan commented 4 months ago

That sounds more like a workaround than a fix.

Alf0nso commented 4 months ago

Yes you are completely right, it is definitely not the fix... I think maybe the "bug" here is that the definition of vterm-tramp-shells is written has:

(defcustom vterm-tramp-shells '(("docker" "/bin/sh"))
  "The shell that gets run in the vterm for tramp.

`vterm-tramp-shells' has to be a list of pairs of the format:
\(TRAMP-METHOD SHELL)"
  :type '(alist :key-type string :value-type string)
  :group 'vterm)

and should be:

(defcustom vterm-tramp-shells '(("docker" "/bin/sh"))
  "The shell that gets run in the vterm for tramp.

`vterm-tramp-shells' has to be a list of pairs of the format:
\(TRAMP-METHOD SHELL)"
  :type '(alist (:key-type string :value-type string))
  :group 'vterm)

At least when I do this change on the source code, this works:

  (use-package vterm
    :ensure t
    :config
    (setopt vterm-tramp-shells '(("ssh" "/bin/bash")
                 ("docker" "/bin/sh")))
    )

But I must confess I have no idea if this is right

Sbozzolo commented 3 months ago

I am not familiar with setopt, if you can provide an example from some other high-profile package that uses this pattern, we can implement the proposed fix to vterm.

Thaodan commented 3 months ago

setopt is basically setf but with check against custom types.

Alf0nso commented 3 months ago

@Sbozzolo I have to confess that I am also not very familiar with setopt, normally I just use setq for everything. Like @Thaodan mentioned, indeed the setopt keyword makes use of both the types (it "type checks" what we pass it) and also a setter function in case it is defined gnu emacs manual. In this specific case when we use setopt we are type checking against:

:type '(alist :key-type string :value-type string)

I played around a little bit more with defcustom and setopt and I think I might regress on my previous suggestion. I would like to ask for @Thaodan to try to write:

(use-package vterm
  :bind (:map vterm-mode-map
              ("C-s" . isearch-forward))
  :config
  (setq vterm-max-scrollback 100000)
  ;; Include the title in vterm and multi-vterm buffers
  ;; setting multi-vterm-buffer-name isn't enough.
  (setq vterm-buffer-name-string "*vterm %s*")
  (setopt vterm-tramp-shells '(("docker" . "/bin/sh")
                               ("ssh" . "/bin/zsh"))))

to see if it works without the warning. If yes, than the only thing that maybe should be changed is the default value of the defcustom variable. Meaning:

(defcustom vterm-tramp-shells '(("docker" "/bin/sh"))

to:

(defcustom vterm-tramp-shells '(("docker" . "/bin/sh"))

akermu / emacs-libvterm

Tramp-login-shells custom doesn't match type #689