Subject: Re: bug#65039: [PATCH] Add bookmark handler for shell-mode

From: Michael Albinus <[email-masked]>
Date: Sat, 15 Feb 2025 18:15:52 +0100

Ship Mints <[email-masked]> writes:

> Latest patch attached.

And still nitpicks :-)

> +Note: Before creating ad-hoc multi-hop remote connections, customize
> +either or both:
> +@code{tramp-save-ad-hoc-proxies} to non-nil to persist proxy routes.
> +@code{tramp-show-ad-hoc-proxies} to non-nil to ensure connections are
> +fully qualified.  This is helpful if you use the same persisted
> +bookmarks file on multiple hosts.

non-@code{nil}

> +@xref{Top, The Tramp Manual,, tramp, The Tramp Manual}. Also see the
> +more detailed documentation available here
> +@url{https://www.gnu.org/software/tramp/}.

??? Both the info manual and the HTML page are the same text.

> +  (let ((bookmark-shell-file-name
> +         (cond
> +          ((file-remote-p default-directory)
> +           (with-connection-local-variables
> +            (cdr (assoc 'shell-file-name (buffer-local-value
> +                                          'connection-local-variables-alist
> +                                          (current-buffer))))))
> +          (shell-file-name shell-file-name)
> +          (t sh-shell-file))))

My previous comment still stands: Use `connection-local-value'. Did you check?

> +(defun shell-bookmark-jump (bookmark)
> +  "Default BOOKMARK handler for shell buffers.
> +Create a shell buffer with its `default-directory', shell process, and
> +buffer name from the bookmark.  If there is an existing shell buffer of
> +the same name, default shell-mode behavior is to reuse that buffer.

`shell-mode'

> +For a remote shell `default-directory' will be the remote file name.
> +Remote shell buffers reuse existing connections that match the remote
> +file name, or may prompt you to create a new connection.  Bind
> +`tramp-show-ad-hoc-proxies' to non-nil to ensure multi-hop remote
> +connections are fully qualified.

non-@code{nil}

Best regards, Michael.