Subject: bug#59326: closed (Re: bug#59326: 29.0.50; [PATCH] Handle bookmark-relocate error)

From: [email-masked] (GNU bug Tracking System)
Date: Sun, 20 Nov 2022 10:18:02 +0000

Your bug report

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

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
59326: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59326
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#59326: 29.0.50; [PATCH] Handle bookmark-relocate error
From: Eli Zaretskii <[email-masked]>
To: Gabriel <[email-masked]>
Cc: [email-masked]
Date: Sun, 20 Nov 2022 12:17:54 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> From: Gabriel <[email-masked]>
> 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.

Subject: 29.0.50; [PATCH] Handle bookmark-relocate error
From: Gabriel <[email-masked]>
To: [email-masked]
Date: Wed, 16 Nov 2022 22:15:40 -0300
List-Id: <debbugs-submit.debbugs.gnu.org>

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. 

>From f528e26047974d044e35828c52c35229705b278b Mon Sep 17 00:00:00 2001
From: Gabriel do Nascimento Ribeiro <[email-masked]>
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

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

---
Gabriel