bug#48179: bookmark-fontify [PATCH]

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

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

The attached patch adds a feature to bookmark.el to colorize a
bookmark's line, making it easily observable. The idea was adopted from
package bm.el, available on MELPA. I was able to test this on Emacs 27,
but there seem to have been some changes in snapshot to make it
difficult to use that version of bookmark.el on my Emacs 27, so please
verify that I performed the migration properly.

OFF-TOPIC, but related: I've posted a pull-request to package bm.el
adding to it a feature to make context-based suggestions for default
bookmark annotations[1]. If Emacs wants that for package bookmark.el it
should be possible to submit a parallel patch.

[1] https://github.com/Boruch-Baum/emacs-bm/commit/dc1d8ad7eea64d94c345f3ed3885eefd8ec72d65

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

[-- Attachment #2: bookmark-fontify.patch --]
[-- Type: text/x-diff, Size: 3470 bytes --]

diff --git a/bookmark.el b/bookmark.el
index 206c872..197678d 100644
--- a/bookmark.el
+++ b/bookmark.el
@@ -165,11 +165,28 @@ A non-nil value may result in truncated bookmark names."
   "Time before `bookmark-bmenu-search' updates the display."
   :type  'number)

+(defcustom bookmark-fontify t
+  "Whether to colorize a bookmark's line.
+See face `bookmark-face'."
+  :type  'boolean
+  :version "27.2")
+
 (defface bookmark-menu-heading
   '((t (:inherit font-lock-type-face)))
   "Face used to highlight the heading in bookmark menu buffers."
   :version "22.1")

+(defface bookmark-face
+  '((((class grayscale)
+      (background light)) (:background "DimGray"))
+    (((class grayscale)
+      (background dark))  (:background "LightGray"))
+    (((class color)
+      (background light)) (:foreground "White" :background "DarkOrange1"))
+    (((class color)
+      (background dark))  (:foreground "Black" :background "DarkOrange1")))
+  "Face used to highlight current line."
+  :version "27.2")

 ;;; No user-serviceable parts beyond this point.

@@ -418,6 +435,31 @@ In other words, return all information but the name."
   "Set the rear-context-string of BOOKMARK-NAME-OR-RECORD to STRING."
   (bookmark-prop-set bookmark-name-or-record 'rear-context-string string))

+(defun bookmark--fontify ()
+  "Apply a colorized overlay to the bookmarked location.
+See defcustom variable `bookmark-fontify'."
+  (let ((bm (make-overlay (point-at-bol)
+                          (min (point-max) (+ 1 (point-at-eol))))))
+    (overlay-put bm 'category 'bookmark)
+    (overlay-put bm 'face 'bookmark-face)))
+
+(defun bookmark--unfontify (bm)
+  "Remove a bookmark's colorized overlay.
+BM is a bookmark as returned from function `bookmark-get-bookmark'.
+See defcustom variable `bookmark-fontify'."
+  (let ((filename (assq 'filename bm))
+        (pos      (assq 'position bm))
+        (buffers  (buffer-list))
+        buf overlays found temp)
+    (when filename (setq filename (expand-file-name (cdr filename))))
+    (when pos (setq pos (cdr pos)))
+    (while (setq buf (pop buffers))
+      (with-current-buffer buf
+        (when (equal filename buffer-file-name)
+          (setq overlays (overlays-at pos))
+          (while (and (not found) (setq temp (pop overlays)))
+            (when (eq 'bookmark (overlay-get temp 'category))
+              (delete-overlay (setq found temp)))))))))

 (defun bookmark-get-handler (bookmark-name-or-record)
   "Return the handler function for BOOKMARK-NAME-OR-RECORD, or nil if none."
@@ -824,7 +866,9 @@ still there, in order, if the topmost one is ever deleted."

            ;; Ask for an annotation buffer for this bookmark
            (when bookmark-use-annotations
-             (bookmark-edit-annotation str))))
+             (bookmark-edit-annotation str))
+           (when bookmark-fontify
+             (bookmark--fontify))))
     (setq bookmark-yank-point nil)
     (setq bookmark-current-buffer nil)))

@@ -1349,6 +1393,7 @@ probably because we were called from there."
   (bookmark-maybe-historicize-string bookmark-name)
   (bookmark-maybe-load-default-file)
   (let ((will-go (bookmark-get-bookmark bookmark-name 'noerror)))
+    (bookmark--unfontify will-go)
     (setq bookmark-alist (delq will-go bookmark-alist))
     ;; Added by db, nil bookmark-current-bookmark if the last
     ;; occurrence has been deleted

bug#48179: [External] : bug#48179: bookmark-fontify [PATCH]

[Drew Adams]

> The attached patch adds a feature to bookmark.el to colorize a
> bookmark's line, making it easily observable. The idea was
> adopted from package bm.el, available on MELPA.

Yet another feature of Bookmark+.

https://www.emacswiki.org/emacs/BookmarkPlus#HighlightingBookmarkLocations

https://www.emacswiki.org/emacs/BookmarkPlus#UsingHighlightedBookmarks

bug#48179: [External] : bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-03 00:40, Drew Adams wrote:
> > The attached patch adds a feature to bookmark.el to colorize a
> > bookmark's line, making it easily observable. The idea was
> > adopted from package bm.el, available on MELPA.
>
> Yet another feature of Bookmark+.

After all these years, I'm still years behind...

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

bug#48179: [External] : bug#48179: bookmark-fontify [PATCH]

[Drew Adams]

> > > The attached patch adds a feature to bookmark.el to colorize a
> > > bookmark's line, making it easily observable. The idea was
> > > adopted from package bm.el, available on MELPA.
> >
> > Yet another feature of Bookmark+.
> 
> After all these years, I'm still years behind...

Same here.  No shame in that.

Bookmark+ features haven't been accepted for Emacs.
You may have better luck.

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> The attached patch adds a feature to bookmark.el to colorize a
> bookmark's line, making it easily observable. The idea was adopted from
> package bm.el, available on MELPA. I was able to test this on Emacs 27,
> but there seem to have been some changes in snapshot to make it
> difficult to use that version of bookmark.el on my Emacs 27, so please
> verify that I performed the migration properly.

The patch is against Emacs 27, I guess?  (Because it didn't apply
cleanly on Emacs 28.)  We're not accepting new features for Emacs 27,
though, so can you respin it for Emacs 28?

Also -- it's slightly obscure what it's doing, but testing seems to
indicate that it just puts a face over the current line when you're
doing a `C-x r m' command?  That's nice, but I think it would also make
sense to restore these faces when jumping to a buffer from the bookmark
buffer -- otherwise it's just a bit confusing.

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

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-03 09:54, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > The attached patch adds a feature to bookmark.el to colorize a
> > bookmark's line, making it easily observable. The idea was adopted from
> > package bm.el, available on MELPA. I was able to test this on Emacs 27,
> > but there seem to have been some changes in snapshot to make it
> > difficult to use that version of bookmark.el on my Emacs 27, so please
> > verify that I performed the migration properly.
>
> The patch is against Emacs 27, I guess?  (Because it didn't apply
> cleanly on Emacs 28.)  We're not accepting new features for Emacs 27,
> though, so can you respin it for Emacs 28?

I thought I did base the patch on yesterday's gnu.org cgit version...
Here's what the on-line log is showing me...

   Age                  Commit message (Expand)                    Author       Files Lines
2020-09-07 Use format-prompt in read-file-name calls that     Lars Ingebrigtsen 1     -3/+3
           have a default
2020-09-06 Use `format-prompt' when prompting with default    Lars Ingebrigtsen 1     -1/+1
           values

> Also -- it's slightly obscure what it's doing, but testing seems to
> indicate that it just puts a face over the current line when you're
> doing a `C-x r m' command?

Just. Yes, that is the entire point: "colorize a bookmark's line".

> That's nice, but I think it would also make sense to restore these
> faces when jumping to a buffer from the bookmark buffer -- otherwise
> it's just a bit confusing.

Yes. If someone deletes the buffer after creating the bookmark, the
overlay will be lost. What you're asking for is a simple two or three
lines to add to the jump function, but there's no sense if you can't
even apply the current patch. If you can figure out what the problem is
with that, I can add the trivial code for the jump.

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

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> I thought I did base the patch on yesterday's gnu.org cgit version...
> Here's what the on-line log is showing me...
>
>    Age                  Commit message (Expand)                    Author       Files Lines
> 2020-09-07 Use format-prompt in read-file-name calls that     Lars Ingebrigtsen 1     -3/+3
>            have a default

There's been half a dozen commits to bookmark.el after that one, so I
think your Emacs is out of date.  The latest is

commit f5b172fb6e41e9bf75acd1fd94325a13d75987bf
Author:     Stefan Kangas <stefan@marxist.se>
AuthorDate: Mon Feb 15 00:43:15 2021 +0100

>> Also -- it's slightly obscure what it's doing, but testing seems to
>> indicate that it just puts a face over the current line when you're
>> doing a `C-x r m' command?
>
> Just. Yes, that is the entire point: "colorize a bookmark's line".

It wasn't clear to me what that meant -- colourise the line in the
bookmark buffer?  Or somewhere else.

> Yes. If someone deletes the buffer after creating the bookmark, the
> overlay will be lost. What you're asking for is a simple two or three
> lines to add to the jump function, but there's no sense if you can't
> even apply the current patch. If you can figure out what the problem is
> with that, I can add the trivial code for the jump.

Great.

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

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

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

On 2021-05-03 11:19, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > I thought I did base the patch on yesterday's gnu.org cgit version...
> > Here's what the on-line log is showing me...
> >
> >    Age                  Commit message (Expand)                    Author       Files Lines
> > 2020-09-07 Use format-prompt in read-file-name calls that     Lars Ingebrigtsen 1     -3/+3
> >            have a default
>
> There's been half a dozen commits to bookmark.el after that one, so I
> think your Emacs is out of date.  The latest is

It's not *my* emacs. I only have access to debian emacs locally; the
data I gave you was from the URL I had for the on-line cgit repository
for emacs, at gnu.org. Here's a second patch, based upon hopefully the
most current URL, and with the additional jump code.

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

[-- Attachment #2: bookmark-fontify-2.patch --]
[-- Type: text/x-diff, Size: 4154 bytes --]

diff --git a/bookmark.el b/bookmark.el
index 98797a0..af91a6e 100644
--- a/bookmark.el
+++ b/bookmark.el
@@ -167,12 +167,29 @@ A non-nil value may result in truncated bookmark names."
   "Time before `bookmark-bmenu-search' updates the display."
   :type  'number)

+(defcustom bookmark-fontify t
+  "Whether to colorize a bookmark's line.
+See face `bookmark-face'."
+  :type  'boolean
+  :version "27.2")
+
 ;; FIXME: No longer used.  Should be declared obsolete or removed.
 (defface bookmark-menu-heading
   '((t (:inherit font-lock-type-face)))
   "Face used to highlight the heading in bookmark menu buffers."
   :version "22.1")

+(defface bookmark-face
+  '((((class grayscale)
+      (background light)) (:background "DimGray"))
+    (((class grayscale)
+      (background dark))  (:background "LightGray"))
+    (((class color)
+      (background light)) (:foreground "White" :background "DarkOrange1"))
+    (((class color)
+      (background dark))  (:foreground "Black" :background "DarkOrange1")))
+  "Face used to highlight current line."
+  :version "27.2")

 ;;; No user-serviceable parts beyond this point.

@@ -427,6 +444,31 @@ In other words, return all information but the name."
 (defvar bookmark-history nil
   "The history list for bookmark functions.")

+(defun bookmark--fontify ()
+  "Apply a colorized overlay to the bookmarked location.
+See defcustom variable `bookmark-fontify'."
+  (let ((bm (make-overlay (point-at-bol)
+                          (min (point-max) (+ 1 (point-at-eol))))))
+    (overlay-put bm 'category 'bookmark)
+    (overlay-put bm 'face 'bookmark-face)))
+
+(defun bookmark--unfontify (bm)
+  "Remove a bookmark's colorized overlay.
+BM is a bookmark as returned from function `bookmark-get-bookmark'.
+See defcustom variable `bookmark-fontify'."
+  (let ((filename (assq 'filename bm))
+        (pos      (assq 'position bm))
+        (buffers  (buffer-list))
+        buf overlays found temp)
+    (when filename (setq filename (expand-file-name (cdr filename))))
+    (when pos (setq pos (cdr pos)))
+    (while (setq buf (pop buffers))
+      (with-current-buffer buf
+        (when (equal filename buffer-file-name)
+          (setq overlays (overlays-at pos))
+          (while (and (not found) (setq temp (pop overlays)))
+            (when (eq 'bookmark (overlay-get temp 'category))
+              (delete-overlay (setq found temp)))))))))

 (defun bookmark-completing-read (prompt &optional default)
   "Prompting with PROMPT, read a bookmark name in completion.
@@ -825,7 +867,9 @@ still there, in order, if the topmost one is ever deleted."

            ;; Ask for an annotation buffer for this bookmark
            (when bookmark-use-annotations
-             (bookmark-edit-annotation str))))
+             (bookmark-edit-annotation str))
+           (when bookmark-fontify
+             (bookmark--fontify))))
     (setq bookmark-yank-point nil)
     (setq bookmark-current-buffer nil)))

@@ -1094,6 +1138,14 @@ and then show any annotations for this bookmark."
     (if win (set-window-point win (point))))
   ;; FIXME: we used to only run bookmark-after-jump-hook in
   ;; `bookmark-jump' itself, but in none of the other commands.
+  (when bookmark-fontify
+    (let ((overlays (overlays-at (point)))
+          temp found)
+      (while (and (not found) (setq temp (pop overlays)))
+        (when (eq 'bookmark (overlay-get temp 'category))
+          (setq found t)))
+      (unless found
+        (bookmark--fontify))))
   (run-hooks 'bookmark-after-jump-hook)
   (if bookmark-automatically-show-annotations
       ;; if there is an annotation for this bookmark,
@@ -1357,6 +1409,7 @@ probably because we were called from there."
   (bookmark-maybe-historicize-string bookmark-name)
   (bookmark-maybe-load-default-file)
   (let ((will-go (bookmark-get-bookmark bookmark-name 'noerror)))
+    (bookmark--unfontify will-go)
     (setq bookmark-alist (delq will-go bookmark-alist))
     ;; Added by db, nil bookmark-current-bookmark if the last
     ;; occurrence has been deleted

bug#48179: bookmark-fontify [PATCH]

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

> Bookmark+ features haven't been accepted for Emacs.
> You may have better luck.

That's too bad, as I would really like to see some (but not all) of its
features in Emacs.

Which features are you talking about more specifically?  Could you show
us the corresponding bug reports?

Thanks in advance.

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> It's not *my* emacs. I only have access to debian emacs locally; the
> data I gave you was from the URL I had for the on-line cgit repository
> for emacs, at gnu.org. Here's a second patch, based upon hopefully the
> most current URL, and with the additional jump code.

Thanks; applied to Emacs 28 without any problems.

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

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thanks; applied to Emacs 28 without any problems.

One thought -- perhaps it should restore all the marked lines (if you
have several bookmarked lines in the same file)?  Hm...  or perhaps
not. 

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

bug#48179: bookmark-fontify [PATCH]

[Basil L. Contovounesios]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Thanks; applied to Emacs 28 without any problems.
>
> One thought -- perhaps it should restore all the marked lines (if you
> have several bookmarked lines in the same file)?  Hm...  or perhaps
> not. 

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?

Thanks,

-- 
Basil

bug#48179: [External] : Re: bug#48179: bookmark-fontify [PATCH]

[Drew Adams]

> > Bookmark+ features haven't been accepted for Emacs.
> > You may have better luck.
> 
> That's too bad, as I would really like to see some 
> (but not all) of its features in Emacs.
> 
> Which features are you talking about more specifically?

Take any features you like.  You're the one
saying you want some, but not all.  The library
is GPL and I've signed papers.  Be my guest -
have at it.

If you take features as is, with no substantive
changes, then I can remove them from Bookmark+
(for Emacs versions that have them).  If not,
so be it.

All of Bookmark+ could be included in Emacs,
superseding bookmark.el (incorporating the
functions Bookmark+ uses from that file).

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-04 11:02, Lars Ingebrigtsen wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
> > Thanks; applied to Emacs 28 without any problems.
>
> One thought -- perhaps it should restore all the marked lines (if you
> have several bookmarked lines in the same file)?  Hm...  or perhaps
> not.

It's do-able, but if Drew's contributions are going to be accepted, that
should take priority, and then if the addition is still necessary, it's
just a matter of iterating over the bookmark list at the point of
finding (re-loading) the file and applying overlays to all the bookmarks
in the file.

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

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

"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?  (You can have several
bookmarks in the same file with different names.)

Yes, that would make sense.

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

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> It's do-able, but if Drew's contributions are going to be accepted, that
> should take priority, 

If Drew submits patches, they'll be evaluated like any other patches,
but experience shows that that's unlikely to happen.  So it's best to
just ignore his comments about how foo+.el already does whatever it is
we're adding to Emacs -- he makes these comments about basically
everything, but virtually never submits anything.

> and then if the addition is still necessary, it's just a matter of
> iterating over the bookmark list at the point of finding (re-loading)
> the file and applying overlays to all the bookmarks in the file.

Sounds good.

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

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-05 10:15, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > It's do-able, but if Drew's contributions are going to be accepted, that
> > should take priority,
>
> If Drew submits patches, they'll be evaluated like any other patches,

My reading of his email was that he was submitting an entire package.

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

bug#48179: bookmark-fontify [PATCH]

[Basil L. Contovounesios]

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

bug#48179: bookmark-fontify [PATCH]

[Bastien]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Boruch Baum <boruch_baum@gmx.com> writes:
>
>> It's not *my* emacs. I only have access to debian emacs locally; the
>> data I gave you was from the URL I had for the on-line cgit repository
>> for emacs, at gnu.org. Here's a second patch, based upon hopefully the
>> most current URL, and with the additional jump code.
>
> Thanks; applied to Emacs 28 without any problems.

Just a quick warning: Org contains two features called "capture" and
"refile".  "Capturing" adds notes or tasks to an Org buffer from other
Emacs buffers.  "Refiling" moves a Org headline under another one,
either in the current Org buffer or in another (declared) Org file.

With this change, the Org refiled/captured headlines get fontified and
it might surprise many users until they find out that fontification is
related to this bookmark feature.

There is also dired and, of course, normal bookmarked files: I guess
some users may be really confused--at least those who don't know to
M-x customize-face RET at point to guess what this is all about.

What do you think?  Shall we turn bookmark-fontify off by default?

I for one do like the feature and customized bookmark-face to suit
my needs, but I wonder about others.

-- 
 Bastien

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

bug#48179: [External] : bug#48179: bookmark-fontify [PATCH]

[Drew Adams]

> > > It's do-able, but if Drew's contributions are going
> > > to be accepted, that should take priority,
> >
> > If Drew submits patches, they'll be evaluated like
> > any other patches,
> 
> My reading of his email was that he was submitting
> an entire package.

To be clear -

1. I've submitted plenty of patches in the past.
   They've usually been ignored.  I don't plan on
   submitting a patch along the lines of what you
   (Boruch) have done for this.

2. File `bookmark+-lit.el' provides the highlighting
   we're talking about wrt Bookmark+.  It depends on
   code in the other Bookmark+ files.  (But they
   don't depend on it, so if you don't want such
   highlighting then you need not load it.

   Someone other than I could either use code from
   `bookmark+-lit.el' to patch Emacs or use its
   features or implementation as inspiration or as
   food for thought.

3. If Emacs maintainers decide to replace
   `bookmark.el' with Bookmark+ then I'll be glad to
   (a) incorporate any `bookmark.el` code that's
   currently used by Bookmark+ into Bookmark+, and
   (b) clean up the Bookmark + code so it doesn't
   bother to be compatible with older Emacs versions
   and it doesn't try to accommodate 3rd-party
   libraries when they might be available.

   The resulting library would be renamed `bookmark',
   (same name we use now, for `bookmark.el') but it
   would comprise multiple files.  File `bookmark+.el'
   would be renamed `bookmark.el', replacing that
   existing file.

   The existing doc is plain-text (in file
   `bookmark+-doc.el').  It would need to be rewritten
   to some extent.  I could help with that.  And I'd
   make myself available to help with questions etc.

#3 would take a while.  But I doubt that maintainers
would agree to it anyway.  They have not, in the past
(though Stefan M. did say at one point that it would
be good for that to happen).

Hope things are clear now.  I'm open to Bookmark+
_replacing_ `bookmark.el', and in that case I'd do
the cleanup work needed for that.  (Someone else
would do the GIT fiddling, committing etc.)  And I'm
open to anyone grabbing and repurposing code from
Bookmark+ piecemeal for vanilla Emacs.  I won't be
doing that, however.

As for the replacement possibility, I'd need some
reassurance that it would actually happen, before
I'd bother to work to make it available.

I'm not going to waste time again working on stuff
that has no chance of inclusion.  Sorry.  Been there.
Contributing - yes, I'm open to that.

If that's not acceptable then, as I say, the code is
there; it's GPL; and I offer it.  You're free to
take any of it you like for Emacs, and I'll even help
you understand, if there are questions.

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-05 13:26, Basil L. Contovounesios wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
>...

Important follow-up:

5) Throughout my prior patches, I had been using variable
   'buffer-file-name' to compare against the value for the 'filename'
   key in bookmarks' alists. That is wrong, in the sense that it's only
   correct for buffers visiting files. The correct comparator seems to
   be the output of function (bookmark-buffer-file-name), see there,
   which will cover all cases of all buffers. With that change, it's
   probably wrong to perform 'expand-file-name' when performing any
   comparison.

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

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-05 17:39, Bastien wrote:
> With this change, the Org refiled/captured headlines get fontified and
> it might surprise many users until they find out that fontification is
> related to this bookmark feature.

What are the steps to reproduce this? I don't use those org features, so
I would be starting at square one.

> There is also dired and, of course, normal bookmarked files: I guess
> some users may be really confused--at least those who don't know to
> M-x customize-face RET at point to guess what this is all about.

I don't understand the confusion scenario. The scenario I imagine is:

  a) User navigates POINT to location of desired bookmark

  b) User creates bookmark

  c) User gets immediate (and prominent) visual feedback

  d) User recalls dictum "Correlation is not causality", coughs
     deeply, and lights up another cigarette.

> What do you think?  Shall we turn bookmark-fontify off by default?

I understand the current emacs policy is not to surprise anyone with new
behavior, so I expect Eli Zaretskii would insist it be off by default.

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

bug#48179: bookmark-fontify [PATCH]

[Eli Zaretskii]

> Date: Wed, 5 May 2021 13:25:08 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 48179@debbugs.gnu.org
> 
> > What do you think?  Shall we turn bookmark-fontify off by default?
> 
> I understand the current emacs policy is not to surprise anyone with new
> behavior, so I expect Eli Zaretskii would insist it be off by default.

As long as people are aware of the policy, I don't even need to
insist.

bug#48179: bookmark-fontify [PATCH]

[Colin Baxter]

>>>>> Bastien  <bzg@gnu.org> writes:

    > Lars Ingebrigtsen <larsi@gnus.org> writes:
    >> Boruch Baum <boruch_baum@gmx.com> writes:
    >> 
    >>> It's not *my* emacs. I only have access to debian emacs locally;
    >>> the data I gave you was from the URL I had for the on-line cgit
    >>> repository for emacs, at gnu.org. Here's a second patch, based
    >>> upon hopefully the most current URL, and with the additional
    >>> jump code.
    >> 
    >> Thanks; applied to Emacs 28 without any problems.

    > Just a quick warning: Org contains two features called "capture"
    > and "refile".  "Capturing" adds notes or tasks to an Org buffer
    > from other Emacs buffers.  "Refiling" moves a Org headline under
    > another one, either in the current Org buffer or in another
    > (declared) Org file.

    > With this change, the Org refiled/captured headlines get fontified
    > and it might surprise many users until they find out that
    > fontification is related to this bookmark feature.

    > There is also dired and, of course, normal bookmarked files: I
    > guess some users may be really confused--at least those who don't
    > know to M-x customize-face RET at point to guess what this is all
    > about.

    > What do you think?  Shall we turn bookmark-fontify off by default?

    > I for one do like the feature and customized bookmark-face to suit
    > my needs, but I wonder about others.

I have turned bookmark-fontify off because I found it too confusing with
a feature of bm-bookmark <https://github.com/joodland/bm>, which I
use. I believe bm-bookmark is popular.

Best wishes,

Colin.

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> My reading of his email was that he was submitting an entire package.

But Emacs is not going to replace foo.el with foo+.el, so it's not a
productive contribution to the discussion.  (We've been over this many
times before in many circumstances.)

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

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> I understand the current emacs policy is not to surprise anyone with new
> behavior, so I expect Eli Zaretskii would insist it be off by default.

There is no such policy.  We add new functionality to Emacs all the
time, and this new functionality changes behaviour, of course.

We only default behaviour to "off" if we think it's going to be
disruptive, and this bookmarking functionality is not disruptive.  (Or
at least it's not meant to be.)

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

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Colin Baxter <m43cap@yandex.com> writes:

> I have turned bookmark-fontify off because I found it too confusing with
> a feature of bm-bookmark <https://github.com/joodland/bm>, which I
> use. I believe bm-bookmark is popular.

Can you describe the confusion in the interaction between this package
and the new fontification? 

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

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> 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).

Would it be possible for you to pull down a the git tree and work from
that?  That would be less work for everybody involved (including
yourself), I'd have thought?

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

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> 5) Throughout my prior patches, I had been using variable
>    'buffer-file-name' to compare against the value for the 'filename'
>    key in bookmarks' alists. That is wrong, in the sense that it's only
>    correct for buffers visiting files. The correct comparator seems to
>    be the output of function (bookmark-buffer-file-name), see there,
>    which will cover all cases of all buffers. With that change, it's
>    probably wrong to perform 'expand-file-name' when performing any
>    comparison.

Right.  Can you submit a patch to fix that?

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

bug#48179: bookmark-fontify [PATCH]

[Colin Baxter]

>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:

    > Colin Baxter <m43cap@yandex.com> writes:
    >> I have turned bookmark-fontify off because I found it too
    >> confusing with a feature of bm-bookmark
    >> <https://github.com/joodland/bm>, which I use. I believe
    >> bm-bookmark is popular.

    > Can you describe the confusion in the interaction between this
    > package and the new fontification?

As I understand it, both "bm-bookmark" and the bookmark-fontify feature
of the built-in "bookmark" use a colored bar in the bookmarked file to
indicate that the file has been bookmarked. Whereas bookmark-fontify
places the bar on the first line of the bookmarked file, bm-bookmark can
place it at any user-selected position. I use both bm-bookmark and the
built-in bookmark for different purposes and I prefer the colorising to
be restricted to the former.

Perhaps I should add in fairness that bm-bookmark does allow other means
of indicating the presence of a bookmark, but the colored bar is the
default.

Best wishes,

Colin.

bug#48179: bookmark-fontify [PATCH]

[Bastien]

Boruch Baum <boruch_baum@gmx.com> writes:

> What are the steps to reproduce this? I don't use those org features, so
> I would be starting at square one.

1. C-x C-f test.org RET
2. C-c [ (to include test.org in the agenda files)
3. Add this contents to the buffer:

   * A headline to refile

   * A headline where to refile it

4. Put the point at the beginning of "A headline to refile"
5. C-c C-w (or M-x org-refile RET)
6. Select "* A headline where to refile it"

Now the refiled headline is fontified with bookmark-face because Org
adds a bookmark to help you go back to the refile location.

>> There is also dired and, of course, normal bookmarked files: I guess
>> some users may be really confused--at least those who don't know to
>> M-x customize-face RET at point to guess what this is all about.
>
> I don't understand the confusion scenario. The scenario I imagine is:
>
>   a) User navigates POINT to location of desired bookmark

I would say that the confusion arises because

1. The user does not do anything regarding bookmarking

2. The buffer get fontified but he does not know why

3. The user cannot guess why it has been fontified because he does not
   know to M-x customize-face RET

> I understand the current emacs policy is not to surprise anyone with new
> behavior, so I expect Eli Zaretskii would insist it be off by default.

I don't insist on anything.

I'm open to turning org-capture-bookmark to nil by default in Org to
solve the issue for Org.

But again, this is not just Org.  I think many people have bookmarks
they don't even remember, e.g. in dired buffers.  If they see such a
fontified file in dired, they will just wonder what happened.

My suggestion would be to interactively ask the user to customize
bookmarks-fontify on the first C-x r m hit, so that he knows what's
in there for him.

-- 
 Bastien

bug#48179: bookmark-fontify [PATCH]

[Colin Baxter]

>>>>> Bastien  <bzg@gnu.org> writes:

    > Boruch Baum <boruch_baum@gmx.com> writes:
    >> What are the steps to reproduce this? I don't use those org
    >> features, so I would be starting at square one.

    > 1. C-x C-f test.org RET 2. C-c [ (to include test.org in the
    > agenda files) 3. Add this contents to the buffer:

    >    * A headline to refile

    >    * A headline where to refile it

    > 4. Put the point at the beginning of "A headline to refile" 5. C-c
    > C-w (or M-x org-refile RET) 6. Select "* A headline where to
    > refile it"

    > Now the refiled headline is fontified with bookmark-face because
    > Org adds a bookmark to help you go back to the refile location.

    >>> There is also dired and, of course, normal bookmarked files: I
    >>> guess some users may be really confused--at least those who
    >>> don't know to M-x customize-face RET at point to guess what this
    >>> is all about.
    >> 
    >> I don't understand the confusion scenario. The scenario I imagine
    >> is:
    >> 
    >> a) User navigates POINT to location of desired bookmark

    > I would say that the confusion arises because

    > 1. The user does not do anything regarding bookmarking

    > 2. The buffer get fontified but he does not know why

    > 3. The user cannot guess why it has been fontified because he does
    > not know to M-x customize-face RET

    >> I understand the current emacs policy is not to surprise anyone
    >> with new behavior, so I expect Eli Zaretskii would insist it be
    >> off by default.

    > I don't insist on anything.

    > I'm open to turning org-capture-bookmark to nil by default in Org
    > to solve the issue for Org.

    > But again, this is not just Org.  I think many people have
    > bookmarks they don't even remember, e.g. in dired buffers.  If
    > they see such a fontified file in dired, they will just wonder
    > what happened.

    > My suggestion would be to interactively ask the user to customize
    > bookmarks-fontify on the first C-x r m hit, so that he knows
    > what's in there for him.

Good idea +1.

Colin

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-06 10:55, Lars Ingebrigtsen wrote:
> Colin Baxter <m43cap@yandex.com> writes:
>
> > I have turned bookmark-fontify off because I found it too confusing with
> > a feature of bm-bookmark <https://github.com/joodland/bm>, which I
> > use. I believe bm-bookmark is popular.
>
> Can you describe the confusion in the interaction between this package
> and the new fontification?

I also use package bm.el. For me, it acts as a replacement of pacakge
bookmark.el (once modified with two features that I've submitted as PRs
to their upstream[1]. I can't speak for Robert, but I find the two
packages totally overlap and so confusing to have in parallel that I
unload one feature whenever I need to use the other libray.

[1] https://github.com/Boruch-Baum/emacs-bm

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

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-06 10:57, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > 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).
>
> Would it be possible for you to pull down a the git tree and work from
> that?

No. This has been something that I've discussed elsewhere on the Emacs forum.

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

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-06 10:58, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > 5) Throughout my prior patches, I had been using variable
> >    'buffer-file-name' to compare against the value for the 'filename'
> >    key in bookmarks' alists. That is wrong, in the sense that it's only
> >    correct for buffers visiting files. The correct comparator seems to
> >    be the output of function (bookmark-buffer-file-name), see there,
> >    which will cover all cases of all buffers. With that change, it's
> >    probably wrong to perform 'expand-file-name' when performing any
> >    comparison.
>
> Right.  Can you submit a patch to fix that?

Basil has taken the lead on this, and I'm waiting on a response from him
with his integration patch before continuing. Once that happens, I'll
suggest to him that patch and one or two other ideas (eg. a setting change
should be respected immediately across all buffers, so it should be
controlled by a toggle or -mode command; popping a bookmark should
fontify the pop).

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

bug#48179: bookmark-fontify [PATCH]

[Colin Baxter]

>>>>> Boruch Baum <boruch_baum@gmx.com> writes:

    > On 2021-05-06 10:55, Lars Ingebrigtsen wrote:
    >> Colin Baxter <m43cap@yandex.com> writes:
    >> 
    >> > I have turned bookmark-fontify off because I found it too
    >> confusing with > a feature of bm-bookmark
    >> <https://github.com/joodland/bm>, which I > use. I believe
    >> bm-bookmark is popular.
    >> 
    >> Can you describe the confusion in the interaction between this
    >> package and the new fontification?

    > I also use package bm.el. For me, it acts as a replacement of
    > pacakge bookmark.el (once modified with two features that I've
    > submitted as PRs to their upstream[1]. I can't speak for Robert,
    > but I find the two packages totally overlap and so confusing to

I can't agree they overlap - bm-bookmark allows multiply bookmarks in
one file. I don't believe that that can be done via the built-in
bookmark.

Best wishes,

Colin.

bug#48179: bookmark-fontify [PATCH]

[Colin Baxter]

Set too hastily. I should have asked does the built-in bookmark allow
multiply bookmarks in a single file to the same degree as bm-bookmark?

Colin.

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-06 11:57, Bastien wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > What are the steps to reproduce this? I don't use those org features, so
> > I would be starting at square one.
>
> ...

Thanks, Bastien. I'll try it.

> Now the refiled headline is fontified with bookmark-face because Org
> adds a bookmark to help you go back to the refile location.

Is that behavior hidden from the user? What I mean is, does the user
know that what org-mode is doing is creating bookmarks? If yes, then the
user already associates the re-file process with creation of a bookmark.

> ....

> 3. The user cannot guess why it has been fontified because he does not
>    know to M-x customize-face RET

You write further down that "many people have bookmarks they don't even
remember". Is this a case of that? I agree with you that would be a
point of confusion. It speaks to a deficiency in my proposal in that it
doesn't include a clearly-identifiable bookmark icon on the fontified
line. The presence of the icon wouldn't help the user remember how or
when the line became bookmarked, but would clearly identify the line as
a bookmark (and if the user is surprised, it's probably called for to
delete the bookmark anyway).

Having a clearly identifiable icon would also mitigate any confusion in
the case of org-refile.

> But again, this is not just Org.  I think many people have bookmarks
> they don't even remember, e.g. in dired buffers.  If they see such a
> fontified file in dired, they will just wonder what happened.

Been there, done that. At some point, years ago, I changed my personal
policy, deleted all my bookmarks, and have since only used them for very
limited purposes. For me, it was the  accumulation of bookmarks that led
to confusion.

> My suggestion would be to interactively ask the user to customize
> bookmarks-fontify on the first C-x r m hit, so that he knows what's
> in there for him.

That would be a cool and very helpful idea for all new emacs features!


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

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-06 10:58, Lars Ingebrigtsen wrote:
> Right.  Can you submit a patch to fix that?

Also in the wings is a version of a feature that I've submitted to
package bm.el[1][2] to offer context-sensitive default suggestions for
bookmark identifiers when creating a bookmark[2].

[1] https://github.com/Boruch-Baum/emacs-bm
[2] https://github.com/joodland/bm/pull/38

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

bug#48179: bookmark-fontify [PATCH]

[Basil L. Contovounesios]

Boruch Baum <boruch_baum@gmx.com> writes:

> 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.
>
> 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

I wouldn't worry, since I only set out to fix a couple of typos - I
don't intend to work on bookmark.el any time soon.

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

Thanks, any chance of a patch? ;)

-- 
Basil

bug#48179: bookmark-fontify [PATCH]

[Basil L. Contovounesios]

Boruch Baum <boruch_baum@gmx.com> writes:

> On 2021-05-06 10:57, Lars Ingebrigtsen wrote:
>> Boruch Baum <boruch_baum@gmx.com> writes:
>>
>> > 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).
>>
>> Would it be possible for you to pull down a the git tree and work from
>> that?
>
> No. This has been something that I've discussed elsewhere on the Emacs forum.

FWIW, even if you can't clone/build all of Emacs 28 locally, you can
still load/evaluate (most of) its version of bookmark.el without too
much trouble, if that helps you when modifying it.

Either way it's still possible to produce a clean patch against v28.

Thanks,

-- 
Basil

bug#48179: bookmark-fontify [PATCH]

[Basil L. Contovounesios]

Boruch Baum <boruch_baum@gmx.com> writes:

> On 2021-05-06 10:58, Lars Ingebrigtsen wrote:
>> Boruch Baum <boruch_baum@gmx.com> writes:
>>
>> > 5) Throughout my prior patches, I had been using variable
>> >    'buffer-file-name' to compare against the value for the 'filename'
>> >    key in bookmarks' alists. That is wrong, in the sense that it's only
>> >    correct for buffers visiting files. The correct comparator seems to
>> >    be the output of function (bookmark-buffer-file-name), see there,
>> >    which will cover all cases of all buffers. With that change, it's
>> >    probably wrong to perform 'expand-file-name' when performing any
>> >    comparison.
>>
>> Right.  Can you submit a patch to fix that?
>
> Basil has taken the lead on this,

He's not taken the lead on this, he's just a very naughty boy.

> and I'm waiting on a response from him
> with his integration patch before continuing.

??? :)

> Once that happens, I'll suggest to him that patch and one or two other
> ideas (eg. a setting change should be respected immediately across all
> buffers, so it should be controlled by a toggle or -mode command;
> popping a bookmark should fontify the pop).

I suggest that you suggest all of that to this list instead ;).

-- 
Basil

bug#48179: bookmark-fontify [PATCH]

[Boruch Baum]

On 2021-05-06 19:52, Basil L. Contovounesios wrote:
> I wouldn't worry, since I only set out to fix a couple of typos - I
> don't intend to work on bookmark.el any time soon.

Not what you did.

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

bug#48179: bookmark-fontify [PATCH]

[Basil L. Contovounesios]

Boruch Baum <boruch_baum@gmx.com> writes:

> On 2021-05-06 19:52, Basil L. Contovounesios wrote:
>> I wouldn't worry, since I only set out to fix a couple of typos - I
>> don't intend to work on bookmark.el any time soon.
>
> Not what you did.

No, that's exactly what I did: I set out to fix a couple of typos.

Once I'd set out to do that, I noticed that the new bookmark-face was
using an old-fashioned face spec syntax and two docstrings referenced
'defcustom' without quoting it where the English phrase "user option" is
more commonly used.

While there, I noticed that bookmark--fontify could use 1+ and
bookmark--unfontify could use dolist, to make it clearer to the next
reader what that loop is doing, and when it will terminate.

Just like the log message promises:

; Fix and simplify last change in bookmark.el.
7d0067f297 2021-05-04 10:54:24 +0100
https://git.sv.gnu.org/cgit/emacs.git/commit/?id=7d0067f297b131c98a5198eb52d49891a83ac5aa

-- 
Basil

bug#48179: bookmark-fontify [PATCH]

[Lars Ingebrigtsen]

Colin Baxter <m43cap@yandex.com> writes:

> I can't agree they overlap - bm-bookmark allows multiply bookmarks in
> one file. I don't believe that that can be done via the built-in
> bookmark.

I may be misunderstanding something, but setting multiple bookmarks in
the same file works without problems with the built-in bookmark.el, as
far as I can tell?  (You just have to give the bookmarks different
names.)

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

bug#48179: bookmark-fontify [PATCH]

[Colin Baxter]

>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:

    > Colin Baxter <m43cap@yandex.com> writes:
    >> I can't agree they overlap - bm-bookmark allows multiply
    >> bookmarks in one file. I don't believe that that can be done via
    >> the built-in bookmark.

    > I may be misunderstanding something, but setting multiple
    > bookmarks in the same file works without problems with the
    > built-in bookmark.el, as far as I can tell?  (You just have to
    > give the bookmarks different names.)

That's true, but, as you wrote, the user has to come up new names. This
is done automatically in bm-bookmarks, which inserts the corresponding
line as the name. The disadvantage is that bm-bookmarks can't easily
bookmark blank lines.

It seems to me that all the bookmarks, bm, built-in (including
emacs-28), bookmark+, have disadvantages. Personally, I would use
bookmark+ if were not for the fact that it writes unprompted to ~/.emacs.

Best wishes,

Colin Baxter.

bug#48179: [External] : bug#48179: bookmark-fontify [PATCH]

[Drew Adams]

>> setting multiple bookmarks in the same file works
>> without problems with the built-in bookmark.el,
>> as far as I can tell?  (You just have to give the
>> bookmarks different names.)

First, a correction of that, assuming I understand
what it wants to say.  You can have any number of
bookmarks in the same file, and even with the same
location in the file.  And they need _NOT_ have
different names.

[This is true for both vanilla bookmark.el and
 Bookmark+.  But in vanilla all but the first
 bookmark with the same name are unused/unavailable.
 With Bookmark+ you can use all bookmarks that have
 the same name.

 For instance, you can have two bookmarks named
 `foo.el', each named for a file named `foo.el',
 where those files are in different directories.
 That's the case for autofile bookmarks, for
 example.]

> That's true, but, as you wrote, the user has
> to come up new names.

See above - the names need NOT be different.

Secondly, if your point is about a user always
needing to come up with names whenever s?he
_wants_ different names (underlining "wants",
because that's not an Emacs requirement):

Sometimes you _want_ to name a bookmark.
Sometimes you might prefer not to have to do that,
and instead just hit a key to set a bookmark -
no prompting for a name, no need to even hit `RET'
to accept a default name at a prompt.

With Bookmark+, by default `C-x x RET' does that.
No prompting - autonaming.

(Bookmarking keys are on a keymap that's bound,
by default, to `C-x x'.  You can instead of course
put that command on a shorter key.)

That's `bmkp-toggle-autonamed-bookmark-set/delete':

  If there is an autonamed bookmark at point, delete
   it, else create one.

  The bookmark created has no region.  Its name is
  formatted according to option
  `bmkp-autoname-bookmark-function'.

  With a prefix arg, delete *ALL* autonamed bookmarks
  for this buffer.

  Non-interactively, act at POSITION, not point.
  If nil, act at point.

So one key to either create or delete an autonamed
bookmark at point.  And a prefix arg deletes all.

> This is done automatically in bm-bookmarks, which
> inserts the corresponding line as the name.

Bookmark+ lets users customize the autonaming, using
option `bmkp-autoname-bookmark-function':

  Function to automatically name a bookmark at point
   (cursor position).

  It should accept a buffer position as its (first)
   argument.
  The name returned should match the application of
  `bmkp-autoname-format' to the buffer name.

The default value of the option does this:

  Return a bookmark name using POSITION and the current
   buffer name.

  The name is composed as follows:
   POSITION followed by a space and then the buffer name.
   The position value is prefixed with zeros to comprise
     9 characters.
   For example, for POSITION value 31416 and current buffer
     `my-buffer', the name returned would be
     `000031416 my-buffer'.

It's trivial to define a function that returns any
kind of name you want, including using the text of
the current line as the name.

The name of an autonamed bookmark matches option
`bmkp-autoname-format', which is a format string.
By default, the format string accepts a buffer name.
The default value is "^[0-9]\\{9\\} %B".  So a
default bookmark name is the position followed by
the buffer name.

> The disadvantage is that bm-bookmarks can't 
> easily bookmark blank lines.

That's a problem only for BM's particular way of
naming.  The default Bookmark+ naming doesn't have
that problem, for example.

What's important is that users themselves can
easily define the automatic naming they want.  And
that they can create bookmarks both by autonaming
and by providing names explicitly.

> It seems to me that all the bookmarks, bm, built-in (including
> emacs-28), bookmark+, have disadvantages. Personally, I would use
> bookmark+ if were not for the fact that it writes unprompted to ~/.emacs.

I don't know what you mean by that.  Could you
elaborate?

1. It never writes to ~/.emacs, unless you've defined
   that as the bookmark file you want to write to.

2. It never writes to your bookmark file unprompted
   unless you've configured it to do so.  You do that
   in the same way as for vanilla bookmark.el: option
   `bookmark-save-flag'.  Set that option to `nil'
   and bookmarks will never be saved except when you
   explicitly ask to save, e.g. when using command
   `bookmark-save'.

bug#48179: [External] : bug#48179: bookmark-fontify [PATCH]

[Colin Baxter]

>>>>> Drew Adams <drew.adams@oracle.com> writes:
    >> It seems to me that all the bookmarks, bm, built-in (including
    >> emacs-28), bookmark+, have disadvantages. Personally, I would use
    >> bookmark+ if were not for the fact that it writes unprompted to
    >> ~/.emacs.

    > I don't know what you mean by that.  Could you elaborate?

    > 1. It never writes to ~/.emacs, unless you've defined that as the
    > bookmark file you want to write to.

Well I can only say what I saw, and bookmark+ did indeed write to
custom-set-variables. It may be that something in ~/.emacs triggered it,
but I don't intend to pursue the matter further.

Best 

bug#48179: [External] : bug#48179: bookmark-fontify [PATCH]

[Drew Adams]

>     >> It seems to me that all the bookmarks, bm, built-in (including
>     >> emacs-28), bookmark+, have disadvantages. Personally, I would use
>     >> bookmark+ if were not for the fact that it writes unprompted to
>     >> ~/.emacs.
> 
>     > I don't know what you mean by that.  Could you elaborate?
> 
>     > 1. It never writes to ~/.emacs, unless you've defined that as the
>     > bookmark file you want to write to.
> 
> Well I can only say what I saw, and bookmark+ did indeed write to
> custom-set-variables. It may be that something in ~/.emacs triggered it,
> but I don't intend to pursue the matter further.

(I mistakenly thought you meant that it writes your
_bookmarks_ to your init file.)

But I think I know what you might be referring to.
And it's a good point.

When your bookmarks are saved, if option
`bmkp-last-as-first-bookmark-file' is non-nil then
its value is updated to the current bookmark file.

Likewise, when you load a bookmark file with
overwriting (i.e. you switch bookmark files), if
that option is non-nil then it's updated to reflect
the new bookmark file.

You can customize that option, but it's also updated
when you save bookmarks or switch bookmark files.

So it's a particular kind of user option.  The doc
string tells you about this unusual behavior (in the
"NOTE" part):

`bmkp-last-as-first-bookmark-file':

  Whether to use the last-used bookmark file as the first used.
  If nil then Emacs always uses the value of `bookmark-default-file'
  as the initial bookmark file, in any given session.

  If non-nil, Emacs uses the last bookmark file you used, in the 
  last Emacs session.  If none was recorded then it uses
  `bookmark-default-file'.  The particular non-nil value must be an
  absolute file name (possibly containing `~') - it is not expanded).

  NOTE: A non-nil option value is overwritten by Bookmark+, so that
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  it becomes the last-used bookmark file.  A nil value is never
  overwritten.

  You can customize this variable.

This is a feature (i.e., by design), not a bug.
But if you customize it to nil then Bookmark+
should never update your init file with its new
value.
___

[It's really unrelated, but I recommend using a
separate `custom-file', and thus not letting
Customize (or code like what I just mentioned)
fiddle with your init file.  Code written by
code is better off relegated to a different file
from one you write code in.]

bug#48179: bookmark-fontify disable by default

[Y. E.]

I _love_ the _idea_  behind the bookmark-fontify feature.
Unfortunately, for me - not-a-pro-emacs-user - it appeared
to be a source of confusion and frustration.
Because of this, I suggest having it disabled by default,
at least until its usability is improved.
That would also be along the lines:
> ... the current emacs policy is not to surprise anyone with new behavior

The following are the issues I encountered:

1. When typing near the colored line, the color doesn't stay
at the bookmarked line only, but spreads with the newly typed text.

2. It takes additional time and effort to understand why the line(s)
is/are there.
  - According to my experience, bookmarks would appear as side information.
  - Supposedly, a mark on a fringe could potentially be a better choice.

3. A mark on a fringe would also allow fixing the issue
of having a too bright default color
(orange with misterioso-theme in my case grabs all the attention).


Suggestions:

1. Disable bookmark-fontify by default
2. Probably not that easy to change, but maybe use fringe beside the
line instead.
3. Maybe, as Bastien suggested:
> ask the user to customize
> bookmarks-fontify on the first C-x r m hit, so that he knows what's
> in there for him.
4. Maybe to change default colors to something less bright.


Thanks

bug#48179: bookmark-fontify disable by default

[Pankaj Jangid]

> 2. Probably not that easy to change, but maybe use fringe beside the
> line instead.

Yes. I like this idea. For a glimpse may have a look at ‘cider’ package
for Clojure. All the compiled forms have marks in the fringe.

And if it is less intrusive, then we can keep it default-on.