(bookmark-make-record): Cease signaling error, return nil instead

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 4b060e02b18e2b5d28afcedc790b5d9af354eb6e..05757309bc1b4f93e5bfb246855d454ceaf28869 100644 (file)
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -508,43 +508,42 @@ See user option `bookmark-fringe-mark'."
     (when (and pos filename)
       (dolist (buf (buffer-list))
         (with-current-buffer buf
-          (let ((bkmk-fname (ignore-errors (bookmark-buffer-file-name))))
-            (when bkmk-fname
-              ;; Normalize both filenames before comparing, because the
-              ;; filename we receive from the bookmark wasn't
-              ;; necessarily generated by `bookmark-buffer-file-name'.
-              ;; For example, bookmarks set in Info nodes get a filename
-              ;; based on `Info-current-file', and under certain
-              ;; circumstances that can be an unexpanded path (e.g.,
-              ;; when the Info page was under your home directory).
-              (let ((this-fname-normalized (expand-file-name filename))
-                    (bkmk-fname-normalized (expand-file-name bkmk-fname)))
-                (when (equal this-fname-normalized bkmk-fname-normalized)
-                  (setq overlays
-                        (save-excursion
-                          (save-restriction
-                            ;; Suppose bookmark "foo" was earlier set at
-                            ;; location X in a file, but now the file is
-                            ;; narrowed such that X is outside the
-                            ;; restriction.  Then the `goto-char' below
-                            ;; would go to the wrong place and thus the
-                            ;; wrong overlays would be fetched.  This is
-                            ;; why we temporarily `widen' before
-                            ;; fetching.
-                            ;;
-                            ;; (This circumstance can easily arise when
-                            ;; a bookmark was set on Info node X but now
-                            ;; the "*info*" buffer is showing some other
-                            ;; node Y, with X and Y physically located
-                            ;; in the same file, as is often the case
-                            ;; with Info nodes.  See bug #70019, for
-                            ;; example.)
-                            (widen)
-                            (goto-char pos)
-                            (overlays-in (pos-bol) (1+ (pos-bol))))))
-                  (while (and (not found) (setq temp (pop overlays)))
-                    (when (eq 'bookmark (overlay-get temp 'category))
-                      (delete-overlay (setq found temp)))))))))))))
+          (when-let ((bkmk-fname (bookmark-buffer-file-name)))
+            ;; Normalize both filenames before comparing, because the
+            ;; filename we receive from the bookmark wasn't
+            ;; necessarily generated by `bookmark-buffer-file-name'.
+            ;; For example, bookmarks set in Info nodes get a filename
+            ;; based on `Info-current-file', and under certain
+            ;; circumstances that can be an unexpanded path (e.g.,
+            ;; when the Info page was under your home directory).
+            (let ((this-fname-normalized (expand-file-name filename))
+                  (bkmk-fname-normalized (expand-file-name bkmk-fname)))
+              (when (equal this-fname-normalized bkmk-fname-normalized)
+                (setq overlays
+                      (save-excursion
+                        (save-restriction
+                          ;; Suppose bookmark "foo" was earlier set at
+                          ;; location X in a file, but now the file is
+                          ;; narrowed such that X is outside the
+                          ;; restriction.  Then the `goto-char' below
+                          ;; would go to the wrong place and thus the
+                          ;; wrong overlays would be fetched.  This is
+                          ;; why we temporarily `widen' before
+                          ;; fetching.
+                          ;;
+                          ;; (This circumstance can easily arise when
+                          ;; a bookmark was set on Info node X but now
+                          ;; the "*info*" buffer is showing some other
+                          ;; node Y, with X and Y physically located
+                          ;; in the same file, as is often the case
+                          ;; with Info nodes.  See bug #70019, for
+                          ;; example.)
+                          (widen)
+                          (goto-char pos)
+                          (overlays-in (pos-bol) (1+ (pos-bol))))))
+                (while (and (not found) (setq temp (pop overlays)))
+                  (when (eq 'bookmark (overlay-get temp 'category))
+                    (delete-overlay (setq found temp))))))))))))
 
 (defun bookmark-sort-by-last-modified-time (names)
   "Sort bookmark NAMES by bookmark last modified time, then alphabetically."
@@ -658,9 +657,8 @@ Modes may set this variable buffer-locally to enable bookmarking of
 locations that should be treated specially, such as Info nodes,
 news posts, images, pdf documents, etc.
 
-The function will be called with no arguments.
-It should signal a user error if it is unable to construct a record for
-the current location.
+The function will be called with no arguments.  It should return nil if
+it is unable to construct a record for the current location.
 
 The returned record should be a cons cell of the form (NAME . ALIST)
 where ALIST is as described in `bookmark-alist' and may typically contain
@@ -674,24 +672,24 @@ equivalently just return ALIST without NAME.")
 
 (defcustom bookmark-inhibit-context-functions nil
   "List of functions to call before making a bookmark record.
-The functions take `buffer-file-name' as argument.  If any of
-these functions returns non-nil, the bookmark does not record
+The functions take the value of variable `buffer-file-name' as argument.
+If any of these functions returns non-nil, the bookmark does not record
 context strings from the current buffer."
   :type 'hook
   :version "29.1")
 
 (defun bookmark-make-record ()
   "Return a new bookmark record (NAME . ALIST) for the current location."
-  (let* ((bookmark-search-size
-          ;; If we're in a buffer that's visiting an encrypted file,
-          ;; don't include any context in the bookmark file, because
-          ;; that would leak (possibly secret) data.
-          (if (and buffer-file-name
-                   (run-hook-with-args-until-success
-                    'bookmark-inhibit-context-functions buffer-file-name))
-              0
-            bookmark-search-size))
-         (record (funcall bookmark-make-record-function)))
+  (when-let* ((bookmark-search-size
+               ;; If we're in a buffer that's visiting an encrypted file,
+               ;; don't include any context in the bookmark file, because
+               ;; that would leak (possibly secret) data.
+               (if (and buffer-file-name
+                        (run-hook-with-args-until-success
+                         'bookmark-inhibit-context-functions buffer-file-name))
+                   0
+                 bookmark-search-size))
+              (record (funcall bookmark-make-record-function)))
     ;; Set up default name if the function does not provide one.
     (unless (stringp (car record))
       (if (car record) (push nil record))
@@ -754,23 +752,25 @@ If NO-CONTEXT is non-nil, do not include the front- and rear-context
 strings in the record -- the position is enough.
 
 If POSN is non-nil, record POSN as the point instead of `(point)'."
-  `(,@(unless no-file `((filename . ,(bookmark-buffer-file-name))))
-    ,@(unless no-context `((front-context-string
-                           . ,(if (>= (- (point-max) (point))
-                                      bookmark-search-size)
-                                  (buffer-substring-no-properties
-                                   (point)
-                                   (+ (point) bookmark-search-size))
-                                  nil))))
-    ,@(unless no-context `((rear-context-string
-                           . ,(if (>= (- (point) (point-min))
-                                      bookmark-search-size)
-                                  (buffer-substring-no-properties
-                                   (point)
-                                   (- (point) bookmark-search-size))
-                                  nil))))
-    (position . ,(or posn (point)))
-    (last-modified . ,(current-time))))
+  (let ((file-name (or no-file (bookmark-buffer-file-name))))
+    (when (or no-file file-name)
+      `(,@(unless no-file `((filename . ,(bookmark-buffer-file-name))))
+        ,@(unless no-context `((front-context-string
+                                . ,(if (>= (- (point-max) (point))
+                                           bookmark-search-size)
+                                       (buffer-substring-no-properties
+                                        (point)
+                                        (+ (point) bookmark-search-size))
+                                     nil))))
+        ,@(unless no-context `((rear-context-string
+                                . ,(if (>= (- (point) (point-min))
+                                           bookmark-search-size)
+                                       (buffer-substring-no-properties
+                                        (point)
+                                        (- (point) bookmark-search-size))
+                                     nil))))
+        (position . ,(or posn (point)))
+        (last-modified . ,(current-time))))))
 
 \f
 ;;; File format stuff
@@ -983,6 +983,7 @@ recently set one becomes the one in effect, but the others are
 still there, in order, if the topmost one is ever deleted."
   (with-current-buffer (or bookmark-current-buffer (current-buffer))
     (let* ((record (cdr (assq-delete-all 'defaults (bookmark-make-record)))))
+      (unless record (error "Cannot bookmark this location"))
       (bookmark-maybe-load-default-file)
       (cond
        ((eq overwrite-or-push nil)
@@ -1192,19 +1193,20 @@ If the buffer is associated with a file or directory, use that name."
 
 (defun bookmark-buffer-file-name ()
   "Return the current buffer's file in a way useful for bookmarks."
-  ;; Abbreviate the path, both so it's shorter and so it's more
-  ;; portable.  E.g., the user's home dir might be a different
-  ;; path on different machines, but "~/" will still reach it.
-  (abbreviate-file-name
-   (cond
-    (buffer-file-name buffer-file-name)
-    ((and (boundp 'dired-directory) dired-directory)
-     (if (stringp dired-directory)
-         dired-directory
-       (car dired-directory)))
-    ((and (boundp 'Info-current-file) (stringp Info-current-file))
-     Info-current-file)
-    (t (error "Buffer not visiting a file or directory")))))
+  (when-let
+      (file-name
+       (cond
+        (buffer-file-name buffer-file-name)
+        ((and (boundp 'dired-directory) dired-directory)
+         (if (stringp dired-directory)
+             dired-directory
+           (car dired-directory)))
+        ((and (boundp 'Info-current-file) (stringp Info-current-file))
+         Info-current-file)))
+    ;; Abbreviate the path, both so it's shorter and so it's more
+    ;; portable.  E.g., the user's home dir might be a different
+    ;; path on different machines, but "~/" will still reach it.
+    (abbreviate-file-name file-name)))
 
 (defvar bookmark--watch-already-asked-mtime nil
   "Mtime for which we already queried about reloading.")