bug#59326: 29.0.50; [PATCH] Handle bookmark-relocate error

bug#59326: 29.0.50; [PATCH] Handle bookmark-relocate error

[Gabriel]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

Description:

Currently, Bookmark does not support relocate (bookmark-relocate) for
Types that do not have a filename (e.g.: Eshell, EWW, Man etc).  We
still need to figure out a good way to let Bookmark Types to provide
their own implementation (suggestions are welcome).  While we don't
teach Bookmark to provide such feature, it should handle these cases and
provide an useful error message for users.  See discussion [1].

Steps:

1) emacs -Q (master f793add1758fd01f678698a90f5847b575f63cf8)

2) Run Eshell: M-x eshell RET

3) Add Eshell to Bookmark list: C-x r m RET

4) Open Bookmark list: C-x r l

5) Try to relocate the Eshell bookmark: R

Result: bookmark-relocate: Wrong type argument: stringp, nil

Patch:

A remediation patch is attached. 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-error-in-bookmark-relocate-when-filename-is-n.patch --]
[-- Type: text/x-diff, Size: 2397 bytes --]

From f528e26047974d044e35828c52c35229705b278b Mon Sep 17 00:00:00 2001
From: Gabriel do Nascimento Ribeiro <gabriel376@hotmail.com>
Date: Wed, 16 Nov 2022 21:59:17 -0300
Subject: [PATCH 1/1] Handle error in bookmark-relocate when filename is nil

* lisp/bookmark.el (bookmark-relocate): Handle error when filename is
  nil.
---
 lisp/bookmark.el | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index b57ad12986..45b116ee00 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -1396,20 +1396,25 @@ bookmark-relocate
   (interactive (list (bookmark-completing-read "Bookmark to relocate")))
   (bookmark-maybe-historicize-string bookmark-name)
   (bookmark-maybe-load-default-file)
-  (let* ((bmrk-filename (bookmark-get-filename bookmark-name))
-         (newloc (abbreviate-file-name
-                  (expand-file-name
-                   (read-file-name
-                    (format "Relocate %s to: " bookmark-name)
-                    (file-name-directory bmrk-filename))))))
-    (bookmark-set-filename bookmark-name newloc)
-    (bookmark-update-last-modified bookmark-name)
-    (setq bookmark-alist-modification-count
-          (1+ bookmark-alist-modification-count))
-    (if (bookmark-time-to-save-p)
+  (let ((bmrk-filename (bookmark-get-filename bookmark-name)))
+    ;; FIXME: Make `bookmark-relocate' support bookmark Types
+    ;; besides files and directories.
+    (unless bmrk-filename
+      (user-error "Cannot relocate bookmark of type \"%s\""
+                  (bookmark-type-from-full-record
+                   (bookmark-get-bookmark bookmark-name))))
+    (let ((newloc (abbreviate-file-name
+                   (expand-file-name
+                    (read-file-name
+                     (format "Relocate %s to: " bookmark-name)
+                     (file-name-directory bmrk-filename))))))
+      (bookmark-set-filename bookmark-name newloc)
+      (bookmark-update-last-modified bookmark-name)
+      (setq bookmark-alist-modification-count
+            (1+ bookmark-alist-modification-count))
+      (when (bookmark-time-to-save-p)
         (bookmark-save))
-    (bookmark-bmenu-surreptitiously-rebuild-list)))
-
+      (bookmark-bmenu-surreptitiously-rebuild-list))))
 
 ;;;###autoload
 (defun bookmark-insert-location (bookmark-name &optional no-history)
-- 
2.34.1


[-- Attachment #3: Type: text/plain, Size: 86 bytes --]


[1] https://mail.gnu.org/archive/html/emacs-devel/2022-11/msg00995.html

---
Gabriel

bug#59326: 29.0.50; [PATCH] Handle bookmark-relocate error

[Eli Zaretskii]

> From: Gabriel <gabriel376@hotmail.com>
> Date: Wed, 16 Nov 2022 22:15:40 -0300
> 
> Currently, Bookmark does not support relocate (bookmark-relocate) for
> Types that do not have a filename (e.g.: Eshell, EWW, Man etc).  We
> still need to figure out a good way to let Bookmark Types to provide
> their own implementation (suggestions are welcome).  While we don't
> teach Bookmark to provide such feature, it should handle these cases and
> provide an useful error message for users.  See discussion [1].
> 
> Steps:
> 
> 1) emacs -Q (master f793add1758fd01f678698a90f5847b575f63cf8)
> 
> 2) Run Eshell: M-x eshell RET
> 
> 3) Add Eshell to Bookmark list: C-x r m RET
> 
> 4) Open Bookmark list: C-x r l
> 
> 5) Try to relocate the Eshell bookmark: R
> 
> Result: bookmark-relocate: Wrong type argument: stringp, nil
> 
> Patch:
> 
> A remediation patch is attached. 

Thanks, installed, and closing the bug.