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; 16+ 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 related	[flat|nested] 16+ 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   ` bug#56896: 29.0.50; [PATCH] " 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; 16+ 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] 16+ 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   ` bug#56896: 29.0.50; [PATCH] " Drew Adams
  1 sibling, 1 reply; 16+ 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] 16+ 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; 16+ messages in thread
From: Drew Adams @ 2022-08-02 20:10 UTC (permalink / raw)
  To: Eli Zaretskii, Jim Porter; +Cc: 56896@debbugs.gnu.org

> 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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 related	[flat|nested] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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
  2022-08-13 21:59             ` bug#56896: 29.0.50; [PATCHv3] " Jim Porter
  1 sibling, 1 reply; 16+ 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 related	[flat|nested] 16+ messages in thread

* bug#56896: 29.0.50; [PATCHv3] Make the bookmark fringe icon look like a bookmark
  2022-08-05  4:41           ` Jim Porter
@ 2022-08-13 21:59             ` Jim Porter
  2022-08-15  6:44               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Jim Porter @ 2022-08-13 21:59 UTC (permalink / raw)
  To: Eli Zaretskii, Lars Ingebrigtsen, 56896

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

On 8/4/2022 9:41 PM, Jim Porter wrote:
> 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 #2: 0001-Make-the-bookmark-fringe-icon-look-like-a-bookmark.patch --]
[-- Type: text/plain, Size: 9733 bytes --]

From c83034e2cc4d29a7ab4a84761a107c53bad6cde8 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'.
(fringe-custom-set-bitmap): New function.

* 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, bookmark--remove-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           | 40 +++++++++++++++++++++++++-------------
 lisp/cus-edit.el           | 21 ++++++++++++++++++++
 lisp/fringe.el             | 12 ++++++++++++
 src/fringe.c               | 15 ++++++++++++++
 7 files changed, 84 insertions(+), 14 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 96079dc106..d336cda674 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 e2bccca4a8..f41398e5b0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2011,6 +2011,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 d0893e932b..7466be32b4 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -181,10 +181,25 @@ 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])
+
+(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 #'fringe-custom-set-bitmap
+  :version "29.1")
 
 ;; FIXME: No longer used.  Should be declared obsolete or removed.
 (defface bookmark-menu-heading
@@ -201,10 +216,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,12 +497,9 @@ 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)
@@ -499,7 +511,7 @@ bookmark--set-fringe-mark
 (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 +627,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 +943,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 +1225,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..d5bae8f66f 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..0c88501298 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
@@ -324,6 +325,17 @@ fringe-columns
     ;; The real implementation is in src/fringe.c.
     ))
 
+(defun fringe-custom-set-bitmap (symbol value)
+  "Set SYMBOL to a fringe bitmap VALUE.
+This sets the `fringe' property on SYMBOL to match that of VALUE,
+and then force all windows to be updated on the next redisplay.
+You should use this for the :set parameter for customization
+options to pick a fringe bitmap."
+  (prog1
+      (set symbol value)
+    (put symbol 'fringe (get value 'fringe))
+    (force-window-update)))
+
 (provide 'fringe)
 
 ;;; fringe.el ends here
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 related	[flat|nested] 16+ messages in thread

* bug#56896: 29.0.50; [PATCHv3] Make the bookmark fringe icon look like a bookmark
  2022-08-13 21:59             ` bug#56896: 29.0.50; [PATCHv3] " Jim Porter
@ 2022-08-15  6:44               ` Lars Ingebrigtsen
  2022-08-16  4:17                 ` Jim Porter
  2022-08-21 16:23                 ` Juri Linkov
  0 siblings, 2 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-15  6:44 UTC (permalink / raw)
  To: Jim Porter; +Cc: 56896, Eli Zaretskii

Jim Porter <jporterbugs@gmail.com> writes:

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

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






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

* bug#56896: 29.0.50; [PATCHv3] Make the bookmark fringe icon look like a bookmark
  2022-08-15  6:44               ` Lars Ingebrigtsen
@ 2022-08-16  4:17                 ` Jim Porter
  2022-08-21 16:23                 ` Juri Linkov
  1 sibling, 0 replies; 16+ messages in thread
From: Jim Porter @ 2022-08-16  4:17 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, 56896-done

On 8/14/2022 11:44 PM, Lars Ingebrigtsen wrote:
> Makes sense to me; please go ahead and push.

Thanks. Pushed as b87400c78b047d242ae188c46c621e0e8a8e69b2.





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

* bug#56896: 29.0.50; [PATCHv3] Make the bookmark fringe icon look like a bookmark
  2022-08-15  6:44               ` Lars Ingebrigtsen
  2022-08-16  4:17                 ` Jim Porter
@ 2022-08-21 16:23                 ` Juri Linkov
  1 sibling, 0 replies; 16+ messages in thread
From: Juri Linkov @ 2022-08-21 16:23 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Jim Porter, Eli Zaretskii, 56896

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





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

end of thread, other threads:[~2022-08-21 16:23 UTC | newest]

Thread overview: 16+ 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-13 21:59             ` bug#56896: 29.0.50; [PATCHv3] " Jim Porter
2022-08-15  6:44               ` Lars Ingebrigtsen
2022-08-16  4:17                 ` Jim Porter
2022-08-21 16:23                 ` Juri Linkov
2022-08-02 20:10   ` bug#56896: 29.0.50; [PATCH] " 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 public 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).