From: Eshel Yaron Date: Thu, 20 Jun 2024 20:07:17 +0000 (+0200) Subject: (bookmark-make-record): Cease signaling error, return nil instead X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=439af23c2a83ec2d3d35e8eac7e75e3637c2d0da;p=emacs.git (bookmark-make-record): Cease signaling error, return nil instead --- diff --git a/lisp/bookmark.el b/lisp/bookmark.el index 4b060e02b18..05757309bc1 100644 --- 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)))))) ;;; 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.")