29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark

  • Done
  • quality assurance status badge
Details
7 participants
  • Drew Adams
  • Eli Zaretskii
  • Jim Porter
  • Juri Linkov
  • Lars Ingebrigtsen
  • Po Lu
  • Stefan Kangas
Owner
unassigned
Submitted by
Jim Porter
Severity
wishlist
J
J
Jim Porter wrote on 2 Aug 2022 21:13
(address . bug-gnu-emacs@gnu.org)
39a51230-2a0f-4eb7-a811-e4509a826f5d@gmail.com
Currently, the bookmark fringe icon is a circle. However, Emacs already
uses a circle to represent breakpoints (as do a lot of other IDEs).
These are usually a different color, but I think it would be nice if the
bookmark fringe icon were a different shape too. This would help
colorblind users, since (depending on their Emacs theme and what kind of
colorblindness they have), it might be hard to distinguish the bookmark
icon from the breakpoint icon.

It would help make the purpose of the indicator more obvious to users
who don't directly use bookmarks. Some packages (including the built-in
org-capture package) set bookmarks automatically, and a user might not
realize that the dot indicates a bookmark, as opposed to some other thing.

Attached are some screenshots showing before/after, plus a patch for
this. I converted the string definition of the bitmap to a vector of
(binary) numbers, since then a reader can see the shape of the icon if
they look carefully.
From 0671b44808a07237a1c183e80c7ba713255c9ee8 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 2 Aug 2022 11:40:43 -0700
Subject: [PATCH] Make the bookmark fringe icon look like a bookmark

* lisp/bookmark.el (bookmark-fringe-mark): Change the bitmap to look
like a bookmark.
---
lisp/bookmark.el | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

Toggle diff (22 lines)
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 30a03e0431..53da501316 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -483,7 +483,14 @@ bookmark-history
"The history list for bookmark functions.")
(define-fringe-bitmap 'bookmark-fringe-mark
- "\x3c\x7e\xff\xff\xff\xff\x7e\x3c")
+ [#b01111110
+ #b01111110
+ #b01111110
+ #b01111110
+ #b01111110
+ #b01111110
+ #b01100110
+ #b01000010])
(defun bookmark--set-fringe-mark ()
"Apply a colorized overlay to the bookmarked location.
--
2.25.1
Attachment: before.png
Attachment: after.png
E
E
Eli Zaretskii wrote on 2 Aug 2022 21:18
(name . Jim Porter)(address . jporterbugs@gmail.com)(address . 56896@debbugs.gnu.org)
83tu6u5u9r.fsf@gnu.org
Toggle quote (21 lines)
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Tue, 2 Aug 2022 12:13:44 -0700
>
> Currently, the bookmark fringe icon is a circle. However, Emacs already
> uses a circle to represent breakpoints (as do a lot of other IDEs).
> These are usually a different color, but I think it would be nice if the
> bookmark fringe icon were a different shape too. This would help
> colorblind users, since (depending on their Emacs theme and what kind of
> colorblindness they have), it might be hard to distinguish the bookmark
> icon from the breakpoint icon.
>
> It would help make the purpose of the indicator more obvious to users
> who don't directly use bookmarks. Some packages (including the built-in
> org-capture package) set bookmarks automatically, and a user might not
> realize that the dot indicates a bookmark, as opposed to some other thing.
>
> Attached are some screenshots showing before/after, plus a patch for
> this. I converted the string definition of the bitmap to a vector of
> (binary) numbers, since then a reader can see the shape of the icon if
> they look carefully.

Why not make the icon customizable, and offer several possible bitmaps
to chose from? Hardcoding a single icon will always annoy someone.

Thanks.
J
J
Jim Porter wrote on 2 Aug 2022 22:05
(name . Eli Zaretskii)(address . eliz@gnu.org)(address . 56896@debbugs.gnu.org)
57ab6ad0-8b1f-ac3c-b675-bc4131d3e0c2@gmail.com
On 8/2/2022 12:18 PM, Eli Zaretskii wrote:
Toggle quote (3 lines)
> Why not make the icon customizable, and offer several possible bitmaps
> to chose from? Hardcoding a single icon will always annoy someone.

Sure, we could make this customizable. What would be a good way to go
about this? I see three options:

1) The status quo: users can already call (define-fringe-bitmap
'bookmark-fringe-bitmap ...) to make the icon whatever they like, though
that obviously requires writing (or copy/pasting) Elisp.

2) Let `bookmark-set-fringe-mark' take a symbol for a bitmap to use for
the mark (it currently takes a boolean). This would solve this immediate
case, but not other similar cases. For example, what if a user wants to
customize the fringe icons in diff-mode?

3) Provide a generic way to select what any fringe bitmap looks like.
I'm not quite sure how this would be implemented, but it would then
allow users to change the appearance of, say, the `left-curly-arrow'
icon. (In the past, I've done this via (1) by just calling
`define-fringe-bitmap' again.)
D
D
Drew Adams wrote on 2 Aug 2022 22:10
RE: [External] : bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
(name . 56896@debbugs.gnu.org)(address . 56896@debbugs.gnu.org)
SJ0PR10MB54883508242E713140AF6626F39D9@SJ0PR10MB5488.namprd10.prod.outlook.com
Toggle quote (3 lines)
> Why not make the icon customizable, and offer several possible bitmaps
> to chose from? Hardcoding a single icon will always annoy someone.

+1.

FWIW, Bookmark+ has had this for a decade.

Two options for this. Uses predefined var `fringe-bitmaps'
for the choice. (But really there should (also?) be a user
option for such a list.)

(defcustom bmkp-light-left-fringe-bitmap 'left-triangle
"Symbol for the left fringe bitmap to use to highlight a bookmark."
:type (cons 'choice (mapcar (lambda (bb) (list 'const bb)) fringe-bitmaps))
:group 'bookmark-plus)

(defcustom bmkp-light-right-fringe-bitmap 'right-triangle
"Symbol for the right fringe bitmap to use to highlight a bookmark."
:type (cons 'choice (mapcar (lambda (bb) (list 'const bb)) fringe-bitmaps))
:group 'bookmark-plus))

E.g., setting `bmkp-light-left-fringe-bitmap' to Jim's
`bookmark-fringe-mark' (symbol) uses that fringe bitmap.


And yes, it would be good to add a bitmap such as Jim
suggested to `fringe-bitmaps'.
P
P
Po Lu wrote on 3 Aug 2022 04:23
Re: bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
(name . Jim Porter)(address . jporterbugs@gmail.com)(address . 56896@debbugs.gnu.org)
87r11ycbfs.fsf@yahoo.com
What build of Emacs are you using?
I see that the background of the fringe bitmap doesn't match the fringe
itself, which is usually a bug.
E
E
Eli Zaretskii wrote on 3 Aug 2022 04:28
(name . Jim Porter)(address . jporterbugs@gmail.com)(address . 56896@debbugs.gnu.org)
83sfme5acq.fsf@gnu.org
Toggle quote (26 lines)
> Cc: 56896@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Tue, 2 Aug 2022 13:05:40 -0700
>
> On 8/2/2022 12:18 PM, Eli Zaretskii wrote:
> > Why not make the icon customizable, and offer several possible bitmaps
> > to chose from? Hardcoding a single icon will always annoy someone.
>
> Sure, we could make this customizable. What would be a good way to go
> about this? I see three options:
>
> 1) The status quo: users can already call (define-fringe-bitmap
> 'bookmark-fringe-bitmap ...) to make the icon whatever they like, though
> that obviously requires writing (or copy/pasting) Elisp.
>
> 2) Let `bookmark-set-fringe-mark' take a symbol for a bitmap to use for
> the mark (it currently takes a boolean). This would solve this immediate
> case, but not other similar cases. For example, what if a user wants to
> customize the fringe icons in diff-mode?
>
> 3) Provide a generic way to select what any fringe bitmap looks like.
> I'm not quite sure how this would be implemented, but it would then
> allow users to change the appearance of, say, the `left-curly-arrow'
> icon. (In the past, I've done this via (1) by just calling
> `define-fringe-bitmap' again.)

What I had in mind was 2).

Not sure if we need a general capability as in 3), but if it can be
implemented cleanly and will be convenient for user options, I don't
see why not.

Thanks.
J
J
Jim Porter wrote on 3 Aug 2022 04:42
(name . Po Lu)(address . luangruo@yahoo.com)(address . 56896@debbugs.gnu.org)
825425fe-331f-11b8-94a4-6f899162ec4c@gmail.com
On 8/2/2022 7:23 PM, Po Lu via Bug reports for GNU Emacs, the Swiss army
knife of text editors wrote:
Toggle quote (4 lines)
> What build of Emacs are you using?
> I see that the background of the fringe bitmap doesn't match the fringe
> itself, which is usually a bug.

I think you're correct that it's a bug, although it looks like a fairly
simple one. Here's the relevant bit from the definition of `bookmark-face':

(((class color)
(background light))
:background "White" :foreground "DarkOrange1")
(((class color)
(background dark))
:background "Black" :foreground "DarkOrange1"))

The :background should probably be removed and replaced with
:distant-foreground (I assume the :background was specified just in case
the default background was too close to DarkOrange1, so we shouldn't
regress that).

I can fix this part at the same time as the rest of this bug. Thanks for
pointing it out, since I probably would have glossed over this entirely
otherwise (I don't usually use the default theme, so I hadn't seen this
issue before).
P
P
Po Lu wrote on 3 Aug 2022 06:31
(name . Jim Porter)(address . jporterbugs@gmail.com)(address . 56896@debbugs.gnu.org)
87les6c5in.fsf@yahoo.com
Jim Porter <jporterbugs@gmail.com> writes:

Toggle quote (5 lines)
> The :background should probably be removed and replaced with
> :distant-foreground (I assume the :background was specified just in
> case the default background was too close to DarkOrange1, so we
> shouldn't regress that).

Phew. I thought it was yet another bug in the fringe bitmap drawing
code, which is usually very tricky to work with.

Toggle quote (5 lines)
> I can fix this part at the same time as the rest of this bug. Thanks
> for pointing it out, since I probably would have glossed over this
> entirely otherwise (I don't usually use the default theme, so I hadn't
> seen this issue before).

That would be great, thanks.
J
J
Jim Porter wrote on 4 Aug 2022 05:24
(name . Eli Zaretskii)(address . eliz@gnu.org)(address . 56896@debbugs.gnu.org)
3b7b3223-3710-c57b-4c75-eb050eec63a9@gmail.com
On 8/2/2022 7:28 PM, Eli Zaretskii wrote:
Toggle quote (15 lines)
>> Cc: 56896@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>> Date: Tue, 2 Aug 2022 13:05:40 -0700
>>
>> 2) Let `bookmark-set-fringe-mark' take a symbol for a bitmap to use for
>> the mark (it currently takes a boolean). This would solve this immediate
>> case, but not other similar cases. For example, what if a user wants to
>> customize the fringe icons in diff-mode?
>>
> What I had in mind was 2).
>
> Not sure if we need a general capability as in 3), but if it can be
> implemented cleanly and will be convenient for user options, I don't
> see why not.

How does this look? I added a new built-in fringe bitmap
('large-circle'), since it should be generally-useful. There are a
couple different fringe bitmaps for breakpoints that could use this, but
I didn't do anything about that in this patch.

I also added a Customize widget to let users pick a fringe bitmap. I'm
not super-familiar with Customize, so I just guessed on how this is
supposed to be defined (I based it on the 'font' widget).

Finally, I adjusted the names of a couple bookmark variables and let
users specify a bitmap (or nil) for 'bookmark-fringe-mark'. Note that
changing this (via Customize or not) doesn't force an update of
already-set bookmark fringe marks. That would be nice to have, but I'd
need to study the code quite a bit more to figure out how to do this.

If this seems about right, I'll add a NEWS entry describing the change
(though I welcome any feedback about how much should go in NEWS; I'm not
100% sure).
Attachment: file
E
E
Eli Zaretskii wrote on 4 Aug 2022 08:53
(address . 56896@debbugs.gnu.org)
83edxw4hzn.fsf@gnu.org
Toggle quote (17 lines)
> Cc: 56896@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Wed, 3 Aug 2022 20:24:24 -0700
>
> > Not sure if we need a general capability as in 3), but if it can be
> > implemented cleanly and will be convenient for user options, I don't
> > see why not.
>
> How does this look? I added a new built-in fringe bitmap
> ('large-circle'), since it should be generally-useful. There are a
> couple different fringe bitmaps for breakpoints that could use this, but
> I didn't do anything about that in this patch.
>
> I also added a Customize widget to let users pick a fringe bitmap. I'm
> not super-familiar with Customize, so I just guessed on how this is
> supposed to be defined (I based it on the 'font' widget).

Reading the code, it LGTM. But I'm not familiar enough with the
Customize parts of the patch, so let's wait a bit for others to chime
in. Lars, WDYT?

Toggle quote (6 lines)
> Finally, I adjusted the names of a couple bookmark variables and let
> users specify a bitmap (or nil) for 'bookmark-fringe-mark'. Note that
> changing this (via Customize or not) doesn't force an update of
> already-set bookmark fringe marks. That would be nice to have, but I'd
> need to study the code quite a bit more to figure out how to do this.

I think we should fix this aspect, yes. So please do try to find the
way of doing it with some kind of :set function.

Toggle quote (4 lines)
> If this seems about right, I'll add a NEWS entry describing the change
> (though I welcome any feedback about how much should go in NEWS; I'm not
> 100% sure).

If there's a detailed enough description in the manual(s), the NEWS
entry can be quite short, just mentioning the new capabilities and
variables. If you don't think this is manual-worthy, the NEWS entry
should be a bit more detailed. But don't worry about that, we will
get to it when you submit the actual text for NEWS.

Thanks.
L
L
Lars Ingebrigtsen wrote on 4 Aug 2022 08:57
(name . Eli Zaretskii)(address . eliz@gnu.org)
87sfmcjy1q.fsf@gnus.org
Eli Zaretskii <eliz@gnu.org> writes:

Toggle quote (4 lines)
> Reading the code, it LGTM. But I'm not familiar enough with the
> Customize parts of the patch, so let's wait a bit for others to chime
> in. Lars, WDYT?

Looks good to me.
S
S
Stefan Kangas wrote on 4 Aug 2022 15:58
control message for bug #56896
(address . control@debbugs.gnu.org)
CADwFkmnKe+EfvfuoZFnzDHXemtKRfjfJ0zVAO8fbHTpWJye8Kg@mail.gmail.com
severity 56896 wishlist
quit
J
J
Jim Porter wrote on 5 Aug 2022 06:41
Re: bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
(address . 56896@debbugs.gnu.org)
7566691d-090e-f380-b395-4d2aa2fdebdb@gmail.com
On 8/3/2022 11:53 PM, Eli Zaretskii wrote:
Toggle quote (13 lines)
>> Cc: 56896@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>> Date: Wed, 3 Aug 2022 20:24:24 -0700
>>
>> Finally, I adjusted the names of a couple bookmark variables and let
>> users specify a bitmap (or nil) for 'bookmark-fringe-mark'. Note that
>> changing this (via Customize or not) doesn't force an update of
>> already-set bookmark fringe marks. That would be nice to have, but I'd
>> need to study the code quite a bit more to figure out how to do this.
>
> I think we should fix this aspect, yes. So please do try to find the
> way of doing it with some kind of :set function.

Ok, I figured out a way to do this. I added a proxy object
('bookmark--fringe-mark') that I can dynamically set the 'fringe'
property on, and then the :set function will update that and the display
code will Just Work. Well, so long as a redisplay is triggered, but I
think happens when you set options via Customize? It worked in my tests,
anyway.

This method feels kind of hacky, but I can't think of a better way, and
it's certainly more feasible than trying to find all the fringe markers
manually. (Given that code can define custom bookmark handler functions,
I'm not even sure that would have been possible...)

Toggle quote (6 lines)
> If there's a detailed enough description in the manual(s), the NEWS
> entry can be quite short, just mentioning the new capabilities and
> variables. If you don't think this is manual-worthy, the NEWS entry
> should be a bit more detailed. But don't worry about that, we will
> get to it when you submit the actual text for NEWS.

I added some documentation and a NEWS entry for the user-facing part of
this (the new option). There might be room to add more documentation though.
Attachment: file
J
J
Jim Porter wrote on 13 Aug 2022 23:59
Re: bug#56896: 29.0.50; [PATCHv3] Make the bookmark fringe icon look like a bookmark
01ebb4e2-4f0d-98c1-2e34-e7f5ea3fdc39@gmail.com
On 8/4/2022 9:41 PM, Jim Porter wrote:
Toggle quote (12 lines)
> Ok, I figured out a way to do this. I added a proxy object
> ('bookmark--fringe-mark') that I can dynamically set the 'fringe'
> property on, and then the :set function will update that and the display
> code will Just Work. Well, so long as a redisplay is triggered, but I
> think happens when you set options via Customize? It worked in my tests,
> anyway.
>
> This method feels kind of hacky, but I can't think of a better way, and
> it's certainly more feasible than trying to find all the fringe markers
> manually. (Given that code can define custom bookmark handler functions,
> I'm not even sure that would have been possible...)

Here's a better version of this patch. Rather than a proxy object, it
just sets the 'fringe' property on the defcustom, which the rest of the
bookmark code can then use like a "regular" fringe bitmap (essentially,
it's just an alias to a real fringe bitmap). I also added a
'fringe-custom-set-bitmap' function that anyone can use as a :set function.

This should be general enough that it could be used wherever anyone
wants to allow users to use Customize to change the fringe bitmap that
gets used for a particular purpose. Potentially, it could even be used
for *every* use of a fringe bitmap. That would let users pick icons they
like for a particular purpose based on their general description (e.g.
'right-triangle'), but they could also independently adjust the bitmaps
(e.g. redefining all the fringe bitmaps to be larger for high DPI
monitors). For the latter case, maybe users could download a package
from ELPA to do that.
Attachment: file
L
L
Lars Ingebrigtsen wrote on 15 Aug 2022 08:44
(name . Jim Porter)(address . jporterbugs@gmail.com)
87pmh2c8f7.fsf@gnus.org
Jim Porter <jporterbugs@gmail.com> writes:

Toggle quote (7 lines)
> Here's a better version of this patch. Rather than a proxy object, it
> just sets the 'fringe' property on the defcustom, which the rest of
> the bookmark code can then use like a "regular" fringe bitmap
> (essentially, it's just an alias to a real fringe bitmap). I also
> added a 'fringe-custom-set-bitmap' function that anyone can use as a
> :set function.

Makes sense to me; please go ahead and push.

Toggle quote (10 lines)
> This should be general enough that it could be used wherever anyone
> wants to allow users to use Customize to change the fringe bitmap that
> gets used for a particular purpose. Potentially, it could even be used
> for *every* use of a fringe bitmap. That would let users pick icons
> they like for a particular purpose based on their general description
> (e.g. 'right-triangle'), but they could also independently adjust the
> bitmaps (e.g. redefining all the fringe bitmaps to be larger for high
> DPI monitors). For the latter case, maybe users could download a
> package from ELPA to do that.

I wonder whether we could usefully fold this stuff into the new icons.el
library. I'm not sure how, though, because the fringe stuff is so low
level. And icons.el is all about graceful degradation, and there's not
much to degrade to in a fringe context.
J
J
Jim Porter wrote on 16 Aug 2022 06:17
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
cd9d8942-d24f-e844-8e01-1526eeb5bb0e@gmail.com
On 8/14/2022 11:44 PM, Lars Ingebrigtsen wrote:
Toggle quote (2 lines)
> Makes sense to me; please go ahead and push.

Thanks. Pushed as b87400c78b047d242ae188c46c621e0e8a8e69b2.
Closed
J
J
Juri Linkov wrote on 21 Aug 2022 18:23
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
865yimm190.fsf@mail.linkov.net
Toggle quote (3 lines)
> And icons.el is all about graceful degradation, and there's not
> much to degrade to in a fringe context.

In a fringe context degradation can be applied when the fringe is disabled.
?