unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
@ 2022-08-02 19:13 Jim Porter
  2022-08-02 19:18 ` Eli Zaretskii
  2022-08-03  2:23 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 12+ messages in thread
From: Jim Porter @ 2022-08-02 19:13 UTC (permalink / raw)
  To: 56896

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

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.

[-- Attachment #2: 0001-Make-the-bookmark-fringe-icon-look-like-a-bookmark.patch --]
[-- Type: text/plain, Size: 922 bytes --]

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

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 #3: before.png --]
[-- Type: image/png, Size: 2884 bytes --]

[-- Attachment #4: after.png --]
[-- Type: image/png, Size: 2876 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-02 19:13 bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark Jim Porter
@ 2022-08-02 19:18 ` Eli Zaretskii
  2022-08-02 20:05   ` Jim Porter
  2022-08-02 20:10   ` Drew Adams
  2022-08-03  2:23 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 2 replies; 12+ messages in thread
From: Eli Zaretskii @ 2022-08-02 19:18 UTC (permalink / raw)
  To: Jim Porter; +Cc: 56896

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-02 19:18 ` Eli Zaretskii
@ 2022-08-02 20:05   ` Jim Porter
  2022-08-03  2:28     ` Eli Zaretskii
  2022-08-02 20:10   ` Drew Adams
  1 sibling, 1 reply; 12+ messages in thread
From: Jim Porter @ 2022-08-02 20:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 56896

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-02 19:18 ` Eli Zaretskii
  2022-08-02 20:05   ` Jim Porter
@ 2022-08-02 20:10   ` Drew Adams
  1 sibling, 0 replies; 12+ messages in thread
From: Drew Adams @ 2022-08-02 20:10 UTC (permalink / raw)
  To: Eli Zaretskii, Jim Porter; +Cc: 56896

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-02 19:13 bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark Jim Porter
  2022-08-02 19:18 ` Eli Zaretskii
@ 2022-08-03  2:23 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-08-03  2:42   ` Jim Porter
  1 sibling, 1 reply; 12+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-08-03  2:23 UTC (permalink / raw)
  To: Jim Porter; +Cc: 56896

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.






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-02 20:05   ` Jim Porter
@ 2022-08-03  2:28     ` Eli Zaretskii
  2022-08-04  3:24       ` Jim Porter
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2022-08-03  2:28 UTC (permalink / raw)
  To: Jim Porter; +Cc: 56896

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-03  2:23 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-08-03  2:42   ` Jim Porter
  2022-08-03  4:31     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 12+ messages in thread
From: Jim Porter @ 2022-08-03  2:42 UTC (permalink / raw)
  To: Po Lu; +Cc: 56896

On 8/2/2022 7:23 PM, Po Lu via Bug reports for GNU Emacs, the Swiss army 
knife of text editors wrote:
> 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).





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-03  2:42   ` Jim Porter
@ 2022-08-03  4:31     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 12+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-08-03  4:31 UTC (permalink / raw)
  To: Jim Porter; +Cc: 56896

Jim Porter <jporterbugs@gmail.com> writes:

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

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-03  2:28     ` Eli Zaretskii
@ 2022-08-04  3:24       ` Jim Porter
  2022-08-04  6:53         ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Jim Porter @ 2022-08-04  3:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 56896

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

On 8/2/2022 7:28 PM, Eli Zaretskii wrote:
>> 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 #2: 0001-Make-the-bookmark-fringe-icon-look-like-a-bookmark.patch --]
[-- Type: text/plain, Size: 6281 bytes --]

From 3391ae8fc41b7e2f8c216f7dc6b231f58fae744f 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

* src/fringe.c (large_circle_bits): New variable.
(standard_bitmaps): Add large_circle_bits.

* lisp/fringe.el (fringe-bitmaps): Add 'large-circle'.

* lisp/cus-edit.el (widget-fringe-bitmap-prompt-value-history): New
variable.
(fringe-bitmap): New widget.

* lisp/bookmark.el (bookmark-set-fringe-mark): Obsolete in favor of...
(bookmark-fringe-mark): ... this.
(bookmark-fringe-mark): Rename this fringe bitmap to...
(bookmark-mark): ... and change it to look like a bookmark.
(bookmark-face): Don't set the ':background' of the face.  Instead,
set ':distant-foreground'.
(bookmark--set-fringe-mark): Consult the 'bookmark-fringe-mark'
option.
---
 lisp/bookmark.el | 33 ++++++++++++++++++++++-----------
 lisp/cus-edit.el | 21 +++++++++++++++++++++
 lisp/fringe.el   |  1 +
 src/fringe.c     | 15 +++++++++++++++
 4 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 30a03e0431..76c7b7df5d 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -181,10 +181,14 @@ bookmark-search-delay
   "Time before `bookmark-bmenu-search' updates the display."
   :type  'number)
 
-(defcustom bookmark-set-fringe-mark t
-  "Whether to set a fringe mark at bookmarked lines."
-  :type  'boolean
-  :version "28.1")
+(define-obsolete-variable-alias 'bookmark-set-fringe-mark
+  'bookmark-fringe-mark "29.1")
+
+(defcustom bookmark-fringe-mark 'bookmark-mark
+  "The fringe bitmap to mark bookmarked lines with.
+If nil, don't display a mark on the fringe."
+  :type '(choice (const nil) fringe-bitmap)
+  :version "29.1")
 
 ;; FIXME: No longer used.  Should be declared obsolete or removed.
 (defface bookmark-menu-heading
@@ -201,10 +205,10 @@ bookmark-face
      :foreground "LightGray")
     (((class color)
       (background light))
-     :background "White" :foreground "DarkOrange1")
+     :foreground "DarkOrange1" :distant-foreground "DarkOrange3")
     (((class color)
       (background dark))
-     :background "Black" :foreground "DarkOrange1"))
+     :foreground "DarkOrange1" :distant-foreground "Orange1"))
   "Face used to highlight current line."
   :version "28.1")
 
@@ -482,24 +486,31 @@ bookmark-update-last-modified
 (defvar bookmark-history nil
   "The history list for bookmark functions.")
 
-(define-fringe-bitmap 'bookmark-fringe-mark
-  "\x3c\x7e\xff\xff\xff\xff\x7e\x3c")
+(define-fringe-bitmap 'bookmark-mark
+  [#b01111110
+   #b01111110
+   #b01111110
+   #b01111110
+   #b01111110
+   #b01111110
+   #b01100110
+   #b01000010])
 
 (defun bookmark--set-fringe-mark ()
   "Apply a colorized overlay to the bookmarked location.
-See user option `bookmark-set-fringe-mark'."
+See user option `bookmark-fringe-mark'."
   (let ((bm (make-overlay (point-at-bol) (1+ (point-at-bol)))))
     (overlay-put bm 'category 'bookmark)
     (overlay-put bm 'evaporate t)
     (overlay-put bm 'before-string
                  (propertize
                   "x" 'display
-                  `(left-fringe bookmark-fringe-mark bookmark-face)))))
+                  `(left-fringe ,bookmark-fringe-mark bookmark-face)))))
 
 (defun bookmark--remove-fringe-mark (bm)
   "Remove a bookmark's colorized overlay.
 BM is a bookmark as returned from function `bookmark-get-bookmark'.
-See user option `bookmark-set-fringe'."
+See user option `bookmark-fringe-mark'."
   (let ((filename (cdr (assq 'filename bm)))
         (pos (cdr (assq 'position bm)))
         overlays found temp)
diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index edc09f3199..ca26922c30 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -4286,6 +4286,27 @@ custom-hook-convert-widget
     (widget-put widget :args args)
     widget))
 
+;;; The `fringe-bitmap' Widget.
+
+(defvar widget-fringe-bitmap-prompt-value-history nil
+  "History of input to `widget-fringe-bitmap-prompt-value'.")
+
+(define-widget 'fringe-bitmap 'symbol
+  "A Lisp fringe bitmap name"
+  :format "%v"
+  :tag "Fringe bitmap"
+  :match (lambda (_widget value) (fringe-bitmap-p value))
+  :completions (apply-partially #'completion-table-with-predicate
+                                obarray #'fringe-bitmap-p 'strict)
+  :prompt-match 'fringe-bitmap-p
+  :prompt-history 'widget-face-prompt-value-history
+  :validate (lambda (widget)
+	      (unless (fringe-bitmap-p (widget-value widget))
+		(widget-put widget
+			    :error (format "Invalid fringe bitmap: %S"
+					   (widget-value widget)))
+		widget)))
+
 ;;; The `custom-group-link' Widget.
 
 (define-widget 'custom-group-link 'link
diff --git a/lisp/fringe.el b/lisp/fringe.el
index 657a73772d..da6812e68d 100644
--- a/lisp/fringe.el
+++ b/lisp/fringe.el
@@ -46,6 +46,7 @@ fringe
   (let ((bitmaps '(question-mark exclamation-mark
 		   left-arrow right-arrow up-arrow down-arrow
 		   left-curly-arrow right-curly-arrow
+		   large-circle
 		   left-triangle right-triangle
 		   top-left-angle top-right-angle
 		   bottom-left-angle bottom-right-angle
diff --git a/src/fringe.c b/src/fringe.c
index bf0b5fde76..5d7c8dca99 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -209,6 +209,20 @@
 static unsigned short right_curly_arrow_bits[] = {
    0x3c, 0x3e, 0x03, 0x27, 0x3f, 0x3e, 0x3c, 0x3e};
 
+/* Large circle bitmap.  */
+/*
+  ........
+  ..xxxx..
+  .xxxxxx.
+  xxxxxxxx
+  xxxxxxxx
+  .xxxxxx.
+  ..xxxx..
+  ........
+*/
+static unsigned short large_circle_bits[] = {
+  0x3c, 0x7e, 0xff, 0xff, 0xff, 0xff, 0x7e, 0x3c};
+
 /* Reverse Overlay arrow bitmap.  A triangular arrow.  */
 /*
   ......xx
@@ -454,6 +468,7 @@ #define FRBITS(bits)  bits, STANDARD_BITMAP_HEIGHT (bits)
   { FRBITS (down_arrow_bits),         8, 0, ALIGN_BITMAP_BOTTOM, 0 },
   { FRBITS (left_curly_arrow_bits),   8, 0, ALIGN_BITMAP_CENTER, 0 },
   { FRBITS (right_curly_arrow_bits),  8, 0, ALIGN_BITMAP_CENTER, 0 },
+  { FRBITS (large_circle_bits),       8, 0, ALIGN_BITMAP_CENTER, 0 },
   { FRBITS (left_triangle_bits),      8, 0, ALIGN_BITMAP_CENTER, 0 },
   { FRBITS (right_triangle_bits),     8, 0, ALIGN_BITMAP_CENTER, 0 },
   { FRBITS (top_left_angle_bits),     8, 0, ALIGN_BITMAP_TOP,    0 },
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-04  3:24       ` Jim Porter
@ 2022-08-04  6:53         ` Eli Zaretskii
  2022-08-04  6:57           ` Lars Ingebrigtsen
  2022-08-05  4:41           ` Jim Porter
  0 siblings, 2 replies; 12+ messages in thread
From: Eli Zaretskii @ 2022-08-04  6:53 UTC (permalink / raw)
  To: Jim Porter, Lars Ingebrigtsen; +Cc: 56896

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

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

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-04  6:53         ` Eli Zaretskii
@ 2022-08-04  6:57           ` Lars Ingebrigtsen
  2022-08-05  4:41           ` Jim Porter
  1 sibling, 0 replies; 12+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-04  6:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Jim Porter, 56896

Eli Zaretskii <eliz@gnu.org> writes:

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark
  2022-08-04  6:53         ` Eli Zaretskii
  2022-08-04  6:57           ` Lars Ingebrigtsen
@ 2022-08-05  4:41           ` Jim Porter
  1 sibling, 0 replies; 12+ messages in thread
From: Jim Porter @ 2022-08-05  4:41 UTC (permalink / raw)
  To: Eli Zaretskii, Lars Ingebrigtsen; +Cc: 56896

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

On 8/3/2022 11:53 PM, Eli Zaretskii wrote:
>> 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...)

> 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 #2: 0001-Make-the-bookmark-fringe-icon-look-like-a-bookmark.patch --]
[-- Type: text/plain, Size: 9627 bytes --]

From 1b3ff7578d1eaaa6c20e457a42def46ea4f3108b 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

* src/fringe.c (large_circle_bits): New variable.
(standard_bitmaps): Add large_circle_bits.

* lisp/fringe.el (fringe-bitmaps): Add 'large-circle'.

* lisp/cus-edit.el (widget-fringe-bitmap-prompt-value-history): New
variable.
(fringe-bitmap): New widget.

* lisp/bookmark.el (bookmark-set-fringe-mark): Obsolete in favor of...
(bookmark-fringe-mark): ... this.
(bookmark-fringe-mark): Rename this fringe bitmap to...
(bookmark-mark): ... and change it to look like a bookmark.
(bookmark--fringe-mark): New variable.
(bookmark-face): Don't set the ':background' of the face.  Instead,
set ':distant-foreground'.
(bookmark--set-fringe-mark, bookmark-store, bookmark--jump-via):
Consult the 'bookmark-fringe-mark' option.

* doc/lispref/customize.texi (Simple Types): Document 'fringe-bitmap'
type.

* doc/lispref/display.texi (Fringe Bitmaps): Mention 'large-circle'.

* etc/NEWS: Announce this change.
---
 doc/lispref/customize.texi |  4 ++++
 doc/lispref/display.texi   |  1 +
 etc/NEWS                   |  5 ++++
 lisp/bookmark.el           | 49 ++++++++++++++++++++++++++------------
 lisp/cus-edit.el           | 21 ++++++++++++++++
 lisp/fringe.el             |  1 +
 src/fringe.c               | 15 ++++++++++++
 7 files changed, 81 insertions(+), 15 deletions(-)

diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 528421bf3b..6ba35cffff 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -672,6 +672,10 @@ Simple Types
 for color names, as well as a sample and a button for selecting a
 color name from a list of color names shown in a @file{*Colors*}
 buffer.
+
+@item fringe-bitmap
+The value must be a valid fringe bitmap name.  The widget provides
+completion.
 @end table
 
 @node Composite Types
diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index ace67fbedb..48f46b4cb6 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -4626,6 +4626,7 @@ Fringe Bitmaps
 Used for different types of fringe cursors.
 
 @item @code{exclamation-mark}, @code{question-mark}
+@itemx @code{large-circle}
 Not used by core Emacs features.
 @end table
 
diff --git a/etc/NEWS b/etc/NEWS
index b88fb63662..219e8124fe 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1979,6 +1979,11 @@ recently set.
 *** When editing a bookmark annotation, 'C-c C-k' will now cancel.
 It is bound to the new command 'bookmark-edit-annotation-cancel'.
 
+---
+*** New option 'bookmark-fringe-mark'.
+This option controls the bitmap used to indicate bookmarks in the
+fringe (or 'nil' to disable showing this marker).
+
 ** Exif
 
 ---
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 30a03e0431..0bdf3bcc58 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -181,10 +181,32 @@ bookmark-search-delay
   "Time before `bookmark-bmenu-search' updates the display."
   :type  'number)
 
-(defcustom bookmark-set-fringe-mark t
-  "Whether to set a fringe mark at bookmarked lines."
-  :type  'boolean
-  :version "28.1")
+(define-fringe-bitmap 'bookmark-mark
+  [#b01111110
+   #b01111110
+   #b01111110
+   #b01111110
+   #b01111110
+   #b01111110
+   #b01100110
+   #b01000010])
+
+(defvar bookmark--fringe-mark nil
+  "A proxy object to use when setting the bookmark fringe mark.
+This holds a `fringe' property that can be updated in-place to
+dynamically change the bitmap used for marking bookmarks.")
+
+(define-obsolete-variable-alias 'bookmark-set-fringe-mark
+  'bookmark-fringe-mark "29.1")
+
+(defcustom bookmark-fringe-mark 'bookmark-mark
+  "The fringe bitmap to mark bookmarked lines with.
+If nil, don't display a mark on the fringe."
+  :type '(choice (const nil) fringe-bitmap)
+  :set (lambda (symbol value)
+         (set symbol value)
+         (put 'bookmark--fringe-mark 'fringe (get value 'fringe)))
+  :version "29.1")
 
 ;; FIXME: No longer used.  Should be declared obsolete or removed.
 (defface bookmark-menu-heading
@@ -201,10 +223,10 @@ bookmark-face
      :foreground "LightGray")
     (((class color)
       (background light))
-     :background "White" :foreground "DarkOrange1")
+     :foreground "DarkOrange1" :distant-foreground "DarkOrange3")
     (((class color)
       (background dark))
-     :background "Black" :foreground "DarkOrange1"))
+     :foreground "DarkOrange1" :distant-foreground "Orange1"))
   "Face used to highlight current line."
   :version "28.1")
 
@@ -482,24 +504,21 @@ bookmark-update-last-modified
 (defvar bookmark-history nil
   "The history list for bookmark functions.")
 
-(define-fringe-bitmap 'bookmark-fringe-mark
-  "\x3c\x7e\xff\xff\xff\xff\x7e\x3c")
-
 (defun bookmark--set-fringe-mark ()
   "Apply a colorized overlay to the bookmarked location.
-See user option `bookmark-set-fringe-mark'."
+See user option `bookmark-fringe-mark'."
   (let ((bm (make-overlay (point-at-bol) (1+ (point-at-bol)))))
     (overlay-put bm 'category 'bookmark)
     (overlay-put bm 'evaporate t)
     (overlay-put bm 'before-string
                  (propertize
                   "x" 'display
-                  `(left-fringe bookmark-fringe-mark bookmark-face)))))
+                  `(left-fringe bookmark--fringe-mark bookmark-face)))))
 
 (defun bookmark--remove-fringe-mark (bm)
   "Remove a bookmark's colorized overlay.
 BM is a bookmark as returned from function `bookmark-get-bookmark'.
-See user option `bookmark-set-fringe'."
+See user option `bookmark-fringe-mark'."
   (let ((filename (cdr (assq 'filename bm)))
         (pos (cdr (assq 'position bm)))
         overlays found temp)
@@ -615,7 +634,7 @@ bookmark-store
         ;; 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-set-fringe-mark
+          (when bookmark-fringe-mark
             (bookmark--remove-fringe-mark bm))
           ;; Modify using the new (NAME . ALIST) format.
           (setcdr bm alist))
@@ -931,7 +950,7 @@ bookmark-set-internal
            ;; Ask for an annotation buffer for this bookmark
            (when bookmark-use-annotations
              (bookmark-edit-annotation str))
-           (when bookmark-set-fringe-mark
+           (when bookmark-fringe-mark
              (bookmark--set-fringe-mark))))
     (setq bookmark-yank-point nil)
     (setq bookmark-current-buffer nil)))
@@ -1213,7 +1232,7 @@ bookmark--jump-via
     (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-set-fringe-mark
+  (when bookmark-fringe-mark
     (let ((overlays (overlays-in (point-at-bol) (1+ (point-at-bol))))
           temp found)
       (while (and (not found) (setq temp (pop overlays)))
diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index edc09f3199..ca26922c30 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -4286,6 +4286,27 @@ custom-hook-convert-widget
     (widget-put widget :args args)
     widget))
 
+;;; The `fringe-bitmap' Widget.
+
+(defvar widget-fringe-bitmap-prompt-value-history nil
+  "History of input to `widget-fringe-bitmap-prompt-value'.")
+
+(define-widget 'fringe-bitmap 'symbol
+  "A Lisp fringe bitmap name"
+  :format "%v"
+  :tag "Fringe bitmap"
+  :match (lambda (_widget value) (fringe-bitmap-p value))
+  :completions (apply-partially #'completion-table-with-predicate
+                                obarray #'fringe-bitmap-p 'strict)
+  :prompt-match 'fringe-bitmap-p
+  :prompt-history 'widget-face-prompt-value-history
+  :validate (lambda (widget)
+	      (unless (fringe-bitmap-p (widget-value widget))
+		(widget-put widget
+			    :error (format "Invalid fringe bitmap: %S"
+					   (widget-value widget)))
+		widget)))
+
 ;;; The `custom-group-link' Widget.
 
 (define-widget 'custom-group-link 'link
diff --git a/lisp/fringe.el b/lisp/fringe.el
index 657a73772d..da6812e68d 100644
--- a/lisp/fringe.el
+++ b/lisp/fringe.el
@@ -46,6 +46,7 @@ fringe
   (let ((bitmaps '(question-mark exclamation-mark
 		   left-arrow right-arrow up-arrow down-arrow
 		   left-curly-arrow right-curly-arrow
+		   large-circle
 		   left-triangle right-triangle
 		   top-left-angle top-right-angle
 		   bottom-left-angle bottom-right-angle
diff --git a/src/fringe.c b/src/fringe.c
index bf0b5fde76..5d7c8dca99 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -209,6 +209,20 @@
 static unsigned short right_curly_arrow_bits[] = {
    0x3c, 0x3e, 0x03, 0x27, 0x3f, 0x3e, 0x3c, 0x3e};
 
+/* Large circle bitmap.  */
+/*
+  ........
+  ..xxxx..
+  .xxxxxx.
+  xxxxxxxx
+  xxxxxxxx
+  .xxxxxx.
+  ..xxxx..
+  ........
+*/
+static unsigned short large_circle_bits[] = {
+  0x3c, 0x7e, 0xff, 0xff, 0xff, 0xff, 0x7e, 0x3c};
+
 /* Reverse Overlay arrow bitmap.  A triangular arrow.  */
 /*
   ......xx
@@ -454,6 +468,7 @@ #define FRBITS(bits)  bits, STANDARD_BITMAP_HEIGHT (bits)
   { FRBITS (down_arrow_bits),         8, 0, ALIGN_BITMAP_BOTTOM, 0 },
   { FRBITS (left_curly_arrow_bits),   8, 0, ALIGN_BITMAP_CENTER, 0 },
   { FRBITS (right_curly_arrow_bits),  8, 0, ALIGN_BITMAP_CENTER, 0 },
+  { FRBITS (large_circle_bits),       8, 0, ALIGN_BITMAP_CENTER, 0 },
   { FRBITS (left_triangle_bits),      8, 0, ALIGN_BITMAP_CENTER, 0 },
   { FRBITS (right_triangle_bits),     8, 0, ALIGN_BITMAP_CENTER, 0 },
   { FRBITS (top_left_angle_bits),     8, 0, ALIGN_BITMAP_TOP,    0 },
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-08-05  4:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 19:13 bug#56896: 29.0.50; [PATCH] Make the bookmark fringe icon look like a bookmark Jim Porter
2022-08-02 19:18 ` Eli Zaretskii
2022-08-02 20:05   ` Jim Porter
2022-08-03  2:28     ` Eli Zaretskii
2022-08-04  3:24       ` Jim Porter
2022-08-04  6:53         ` Eli Zaretskii
2022-08-04  6:57           ` Lars Ingebrigtsen
2022-08-05  4:41           ` Jim Porter
2022-08-02 20:10   ` Drew Adams
2022-08-03  2:23 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-08-03  2:42   ` Jim Porter
2022-08-03  4:31     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).