bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-05 13:26, Basil L. Contovounesios wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> > "Basil L. Contovounesios" <contovob@tcd.ie> writes:
> >
> >> If you 'C-x r m' more than once in the same file, the first overlay
> >> remains even after deleting the bookmark.  Shouldn't the first overlay
> >> be deleted when its bookmark position is overwritten?
> >
> > You mean when giving the bookmark the same name?
>
> Yes, sorry, I meant multiple 'C-x r m RET' in the same buffer.
>
> > (You can have several bookmarks in the same file with different
> > names.)
> >
> > Yes, that would make sense.
>
> Thanks,

Basil, I went to start doing more work on this and see you made a commit
in the interim so let me coordinate before we end up cross-commiting
(For me, this is especially important since I'm working with debian v27.1
locally and applying patches to v28 which you seem to using locally).

1) For function bookmark--unfontify

   If more than one buffer in the list can be acted upon, then variable
   'found' @line ~472 should be set nil at this point. If only one
   buffer in the list can be acted upon, then @line ~469 should either
   check variable 'found' or @line ~476 should set the list nil, both
   easier using the prior pop idiom than the dolist idiom.

2) For function bookmark-delete-all

   This seems to be a new function in v28, that I don't have locally. I
   see it when performing a diff. Because it takes a (reasonable and
   efficient) 'short-cut' method, it won't delete any fontified
   overlays, so it needs a variation of function bookmark--unfontify:

   (defun bookmark--unfontify-all ()
     "Remove all bookmarks' colorized overlays.
   This function is meant to be called by function
   `bookmark-delete-all'."
     (dolist (buf (buffer-list))
       (with-current-buffer buf
         (save-restriction
           (widen)
           (remove-overlays (point-min) (point-max) 'category 'bookmark)))))

3) For function bookmark-set-internal

   Additional logic is required in order to account for argument
   variable  'overwrite-or-push'. If I'm understanding those features
   correctly, @lines ~875-878 should be amended to something like:

            ((eq overwrite-or-push 'overwrite)
             (when (setq prior-bm (bookmark-get-bookmark str t))
               (bookmark--unfontify prior-bm))
             (bookmark-store str (cdr record) nil))
            ((eq overwrite-or-push 'push)
             (when (setq prior-bm (bookmark-get-bookmark str t))
               (bookmark--unfontify prior-bm))
             (bookmark-store str (cdr record) t))

   For the above snippet, variable 'prior-bm' should be defined locally
   scoped @line ~861

4) I was expecting to find some sort of bookmark hook function in
   variable 'find-file-hooks'. The idea would be thatd when a file is
   read it's stored bookmarks are immediately fontified.

   (defun bookmark--find-file-hook-function ()
     "Fontify all bookmarks in newly found file."
     (let (pos overlay)
       (dolist (bm  (mapcar 'cdr bookmark-alist))
         (when (and (equal buffer-file-name
                           (expand-file-name (or (cdr (assq 'filename bm)) "")))
                    (setq pos (cdr (assq 'position bm))))
           (save-excursion
             (goto-char pos)
             (setq overlay (make-overlay (point-at-bol)
                                         (min (point-max) (1+ (point-at-eol)))))
             (overlay-put overlay 'category 'bookmark)
             (overlay-put overlay 'face 'bookmark-face))))))

That should go some way to account for your catch and some other
scenarios. There may be others that crop up.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0