bug#49725: [PATCH] Unfontify previous location when overwriting bookmark

bug#49725: [PATCH] Unfontify previous location when overwriting bookmark

[Christopher League]

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


This issue with ‘bookmark-fontify’ came up (within a broader
discussion) on the list in May, in particular by Bastien in
+message-id:<8735v0w5fv.fsf@bzg.fr>. It's obtrusive with the way
‘org-refile’ and ‘org-capture’ use bookmarks, but still bothersome
outside of org. It's not yet fixed in current master.

To reproduce, make sure ‘bookmark-fontify’ is t (the default in
master) and ‘bookmark-face’ is noticeable (orange background by
default). Then:

    emacs -Q
    ;; Visit any file with >20 lines, e.g.:
    M-x find-library RET bookmark RET
    ;; Set bookmark to current line
    C-x r m RET
    ;; Go elsewhere and overwrite same bookmark:
    C-u C-u C-n
    C-x r m RET

When we set or jump to a bookmark, that line gets fontified in the
buffer. Move down a few lines, and set the bookmark again with the
same name. This will (by default) overwrite the previous location, but
now *both* locations are displayed using bookmark-face. When a
bookmark is deleted with ‘bookmark-delete’ or within
‘bookmark-bmenu-list’ (C-x r l), its location gets unfontified. But
when just overwritten, the font overlay remains.

In the fix, I tried to split and edit the nearby comment for clarity
as well. Feedback welcome, thanks.

CL


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-When-bookmark-is-overwritten-unfontify-its-previous-.patch --]
[-- Type: text/x-patch, Size: 1698 bytes --]

From cd1539cb298372f47dc8160ff271e4d8a6f96012 Mon Sep 17 00:00:00 2001
From: Christopher League <league@contrapunctus.net>
Date: Sat, 24 Jul 2021 18:17:07 -0400
Subject: [PATCH] When bookmark is overwritten, unfontify its previous position

* lisp/bookmark.el (bookmark-store): When the bookmark-fontify option
is non-nil, setting or jumping to bookmarks will colorize them using
`bookmark-face'.  With this change, overwriting a bookmark will remove
the fontification at its former position.
---
 lisp/bookmark.el | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 52b96fd203..ff9b8ab138 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -561,10 +561,14 @@ old one."
     (set-text-properties 0 (length stripped-name) nil stripped-name)
     (if (and (not no-overwrite)
              (bookmark-get-bookmark stripped-name 'noerror))
-        ;; already existing bookmark under that name and
-        ;; no prefix arg means just overwrite old bookmark
-        ;; Use the new (NAME . ALIST) format.
-        (setcdr (bookmark-get-bookmark stripped-name) alist)
+        ;; Already existing bookmark under that name and
+        ;; no prefix arg means just overwrite old bookmark.
+        (let ((bm (bookmark-get-bookmark stripped-name)))
+          ;; First clean up if previously location was fontified.
+          (when bookmark-fontify
+            (bookmark--unfontify bm))
+          ;; Modify using the new (NAME . ALIST) format.
+          (setcdr bm alist))
 
       ;; otherwise just cons it onto the front (either the bookmark
       ;; doesn't exist already, or there is no prefix arg.  In either
-- 
2.31.1

bug#49725: [PATCH] Unfontify previous location when overwriting bookmark

[Lars Ingebrigtsen]

Christopher League <league@contrapunctus.net> writes:

> In the fix, I tried to split and edit the nearby comment for clarity
> as well. Feedback welcome, thanks.

Thanks; looks good to me (and pushed to Emacs 28).  Does this also fix
the related bug#49645?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49725: [PATCH] Unfontify previous location when overwriting bookmark

[Christopher League]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thanks; looks good to me (and pushed to Emacs 28). Does this
> also fix the related bug#49645?

Oh, I missed that! Found the discussion from May, but not that
report a few days ago.

I think it must fix that, though Marius describes that the
highlighting gets stuck "when deleting bookmarks", which I think
was working before my patch, which only applies to overwrites.
From what I can tell, the call to ‘bookmark--unfontify’ inside
‘bookmark-delete’ dates to 4 May, in the same commit when
‘bookmark-fontify’ option was introduced.

But maybe he was accumulating bookmarks due to overwrites and
misattributed it. I'll reply on that report to confirm.

CL