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

From: Ship Mints <[email-masked]>
Date: Thu, 13 Feb 2025 11:34:20 -0500

On Thu, Feb 13, 2025 at 11:23 AM Michael Albinus <[email-masked]>
wrote:

> Ship Mints <[email-masked]> writes:
>
> > +(defcustom shell-bookmark-name-function
> #'shell-bookmark-name-from-default-directory
> > +  "Function to generate a shell bookmark name.
> > +The default is `shell-bookmark-name', which see."
> > +  :group 'shell
> > +  :type `(choice (function-item
> ,#'shell-bookmark-name-from-default-directory)
> > +                 (function-item ,#'shell-bookmark-name-from-buffer-name)
> > +                 function)
>
> Wouldn't this be sufficient?
>
> --8<---------------cut here---------------start------------->8---
>   :type '(choice (function-item shell-bookmark-name-from-default-directory)
>                  (function-item shell-bookmark-name-from-buffer-name)
>                  function)
> --8<---------------cut here---------------end--------------->8---
>

It would be but referencing functions as functions vs. naked symbols seems
like a better style?

> > +  (replace-regexp-in-string "\\(.*\\)<[[:digit:]]+>\\'"
> > +                            "\\1"
> > +                            (buffer-name)))
>
> --8<---------------cut here---------------start------------->8---
>   (replace-regexp-in-string "<[[:digit:]]+>\\'" "" (buffer-name)))
> --8<---------------cut here---------------end--------------->8---
>

I'll review that regexp. Thanks for that simplification.

> A further remark (don't know where to do it in your code): If you
> bookmark a remote file name, I recommend to keep multi-hop file names
> (let-bind tramp-show-ad-hoc-proxies to t). Otherwise, a remote file name
> like "/ssh:host|sudo:host:/" would be saved in your bookmark-default-file
> as "/sudo:host:/", which doesn't work in the next Emacs session.
>

I'll experiment with multi-hop now and ensure they are correctly stored and
restored.

-Stephane