GNU bug report logs - #77715
[PATCH] Add ring-bell functions for mode line and header line.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 77715 in the body.
You can then email your comments to 77715 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add ring-bell functions for mode line and header line.
Date: Thu, 10 Apr 2025 11:55:20 -0600

[Message part 1 (text/plain, inline)]

Tags: patch

This feature flashes the current mode-line face (mode-line-active) once,
this is intended to be used in `ring-bell-function', but it can be used
for other purposes.

There have been third-party packages that make this, however many of
them diverge either in features or in faces.

I think Emacs should have had this built-in.

[0001-Add-ring-bell-functions-for-mode-line-and-header-lin.patch (text/patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

Message #8 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 77715 <at> debbugs.gnu.org
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Thu, 10 Apr 2025 14:06:09 -0400

[Message part 1 (text/plain, inline)]

On Thu, Apr 10, 2025 at 1:56 PM Elijah Gabe Pérez <eg642616 <at> gmail.com>
wrote:

> Tags: patch
>
> This feature flashes the current mode-line face (mode-line-active) once,
> this is intended to be used in `ring-bell-function', but it can be used
> for other purposes.
>
> There have been third-party packages that make this, however many of
> them diverge either in features or in faces.
>
> I think Emacs should have had this built-in.
>

An implementation like the one below protects from erroneously leaving
behind remapped faces:

  (defun my/flash-faces (interval faces &rest specs)
    (let ((cookies (mapcar
                    (lambda (face) (face-remap-add-relative face specs))
                    faces)))
      (unwind-protect
          (sit-for interval)
        (mapc #'face-remap-remove-relative cookies))))

  (defun my/ring-bell-function ()
    (my/flash-faces my:mode-line-ring-bell-interval ; I use 0.05
                    my:mode-line-ring-bell-faces ; I use '(internal-border
tab-bar mode-line-active mode-line-inactive)
                    :background my:mode-line-ring-bell-backround)) ; I use
"Firebrick" (I could have used the foreground from 'error)

  (setq ring-bell-function (if my:mode-line-ring-bell-interval ; I allow my
users to disable this
                               #'my/ring-bell-function
                             nil))

[Message part 2 (text/html, inline)]

Message #11 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>,
 "77715 <at> debbugs.gnu.org" <77715 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#77715: [PATCH] Add ring-bell functions for mode
 line and header line.
Date: Thu, 10 Apr 2025 18:40:08 +0000

> There have been third-party packages that make this, however many of
> them diverge either in features or in faces.
> 
> I think Emacs should have had this built-in.

One alternative (it could be built-in):

https://www.emacswiki.org/emacs/download/echo-bell.el

It uses a defcustom instead of a defface, to define the background color for the string shown. The string is also a defcustom: "♪♪♪♪♪♪♪♪♪" by default. A third option is the number of seconds to show the string.

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Drew Adams via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: "77715 <at> debbugs.gnu.org" <77715 <at> debbugs.gnu.org>,
 Ship Mints <shipmints <at> gmail.com>, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Thu, 10 Apr 2025 12:53:46 -0600

Drew Adams via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

>> There have been third-party packages that make this, however many of
>> them diverge either in features or in faces.
>> 
>> I think Emacs should have had this built-in.
>
> One alternative (it could be built-in):
>
> https://www.emacswiki.org/emacs/download/echo-bell.el
>
> It uses a defcustom instead of a defface, to define the background color for the string shown. The string is also a defcustom: "♪♪♪♪♪♪♪♪♪" by default. A third option is the number of seconds to show the string.

I'm not sure about using the echo area, when working with multiple
windows (and on large screens), the eyes tend to be more focused on
where the cursor or window is.

Another alternative would be flash other faces too, such as Ship config
does.  I have it almost ready.

-- 
                                          - E.G via GNU Emacs and Org.

Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>, "Drew Adams via
 Bug reports for GNU Emacs, the Swiss army knife of text editors"
 <bug-gnu-emacs <at> gnu.org>
Cc: "77715 <at> debbugs.gnu.org" <77715 <at> debbugs.gnu.org>,
 Ship Mints <shipmints <at> gmail.com>
Subject: RE: [External] : Re: bug#77715: [PATCH] Add ring-bell functions for
 mode line and header line.
Date: Thu, 10 Apr 2025 21:46:52 +0000

> I'm not sure about using the echo area, when working with multiple
> windows (and on large screens), the eyes tend to be more focused on
> where the cursor or window is.

When Emacs dings it's typically due to a user error.

Where do you see the error message? In the echo area.

Putting the attention-grabbing "flash" or whatever
in the echo area draws your attention to it, where
you see the msg telling you what happened.

Why draw or keep the user's attention _away_ from
that message? The user error might have little or
nothing to do with the text at the cursor of the
selected window.

Draw the user's attention _from_ there to the helpful
message.

Message #26 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Drew Adams <drew.adams <at> oracle.com>, "eg642616 <at> gmail.com"
 <eg642616 <at> gmail.com>, "77715 <at> debbugs.gnu.org" <77715 <at> debbugs.gnu.org>
Cc: "shipmints <at> gmail.com" <shipmints <at> gmail.com>
Subject: RE: [External] : bug#77715: [PATCH] Add ring-bell functions for mode
 line and header line.
Date: Thu, 10 Apr 2025 21:57:37 +0000

I wrote:

> When Emacs dings it's typically due to a user error.
> 
> Where do you see the error message? In the echo area.
> 
> Putting the attention-grabbing "flash" or whatever
> in the echo area draws your attention to it, where
> you see the msg telling you what happened.
> 
> Why draw or keep the user's attention _away_ from
> that message? The user error might have little or
> nothing to do with the text at the cursor of the
> selected window.
> 
> Draw the user's attention _from_ there to the helpful
> message.

`visible-bell' has been around forever. Flashing
the entire frame (or as the Emacs manual says, "the
whole screen") is overkill, and it doesn't direct
your attention to the error message. Similar problem
with the alternatives I think you're considering,
even if they're less overkill than `visible-bell'
behavior.

A modest attention-grabber, out of the way in the 
echo area is better, IMO.

Message #29 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Drew Adams via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: "77715 <at> debbugs.gnu.org" <77715 <at> debbugs.gnu.org>,
 "shipmints <at> gmail.com" <shipmints <at> gmail.com>,
 Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Thu, 10 Apr 2025 21:01:33 -0600

[Message part 1 (text/plain, inline)]

Drew Adams via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> I wrote:
>
>> When Emacs dings it's typically due to a user error.
>> 
>> Where do you see the error message? In the echo area.
>> 
>> Putting the attention-grabbing "flash" or whatever
>> in the echo area draws your attention to it, where
>> you see the msg telling you what happened.
>> 
>> Why draw or keep the user's attention _away_ from
>> that message? The user error might have little or
>> nothing to do with the text at the cursor of the
>> selected window.
>> 
>> Draw the user's attention _from_ there to the helpful
>> message.
>
> `visible-bell' has been around forever. Flashing
> the entire frame (or as the Emacs manual says, "the
> whole screen") is overkill, and it doesn't direct
> your attention to the error message. Similar problem
> with the alternatives I think you're considering,
> even if they're less overkill than `visible-bell'
> behavior.
>
> A modest attention-grabber, out of the way in the 
> echo area is better, IMO.

I think these features could coexist together, it wouldn't hurt.

Flash the mode (or header) line for bell it is something that people
have wanted to have, see packages such as mode-line-bell, doom-themes
(both in MELPA), and nano-bell.

I made my own implementation of echo-bell:

[0001-Add-ring-bell-function-for-flash-faces.-bug-77715.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

Message #35 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: "Drew Adams via Bug reports for GNU Emacs,
 the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>,
 "77715 <at> debbugs.gnu.org" <77715 <at> debbugs.gnu.org>,
 Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Fri, 11 Apr 2025 08:00:43 -0400

[Message part 1 (text/plain, inline)]

On Thu, Apr 10, 2025 at 11:01 PM Elijah Gabe Pérez <eg642616 <at> gmail.com>
wrote:

> Drew Adams via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs <at> gnu.org> writes:
>
> > I wrote:
> >
> >> When Emacs dings it's typically due to a user error.
> >>
> >> Where do you see the error message? In the echo area.
> >>
> >> Putting the attention-grabbing "flash" or whatever
> >> in the echo area draws your attention to it, where
> >> you see the msg telling you what happened.
> >>
> >> Why draw or keep the user's attention _away_ from
> >> that message? The user error might have little or
> >> nothing to do with the text at the cursor of the
> >> selected window.
> >>
> >> Draw the user's attention _from_ there to the helpful
> >> message.
> >
> > `visible-bell' has been around forever. Flashing
> > the entire frame (or as the Emacs manual says, "the
> > whole screen") is overkill, and it doesn't direct
> > your attention to the error message. Similar problem
> > with the alternatives I think you're considering,
> > even if they're less overkill than `visible-bell'
> > behavior.
> >
> > A modest attention-grabber, out of the way in the
> > echo area is better, IMO.
>
> I think these features could coexist together, it wouldn't hurt.
>
> Flash the mode (or header) line for bell it is something that people
> have wanted to have, see packages such as mode-line-bell, doom-themes
> (both in MELPA), and nano-bell.
>
> I made my own implementation of echo-bell:


I think the term duration is better than length when describing time
intervals.

I still prefer the unwind-protected sit-for method that guarantees the
remaps are removed to the one with the timer where I've experienced errors
intervening with a race condition creating new cookies that are not
removed.  Try mashing C-g with your timer implementation and you'll see it.

I also prefer the implementation that I proposed that takes a list of faces
to flash rather than hard-code an assumption about what users want to
flash.  Then this isn't tied to the mode line if that's not what people
want.  Is a new face really necessary just to implement a flash?  The
implementation I proposed allows the user to specify any face attributes to
define what they want for a flash.  I wouldn't use this feature without it
being more reliable and flexible.

[Message part 2 (text/html, inline)]

Message #41 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Ship Mints <shipmints <at> gmail.com>
Cc: 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Fri, 11 Apr 2025 12:18:14 -0600

[Message part 1 (text/plain, inline)]

Ship Mints <shipmints <at> gmail.com> writes:
> I think the term duration is better than length when describing time intervals.

I agree.

> I still prefer the unwind-protected sit-for method that guarantees the remaps are removed to the one with
> the timer where I've experienced errors intervening with a race condition creating new cookies that are
> not removed.  Try mashing C-g with your timer implementation and you'll see it.

The problem with using sit-for is that it delays the message displayed
(usually "Quit") until timer stops (or if there is an input), it look
like Emacs froze.

Anyway, it changed it for use sit-for instead.

> Is a new face really necessary just to implement a flash?  The implementation I proposed
> allows the user to specify any face attributes to define what they want for a flash.  I wouldn't use this
> feature without it being more reliable and flexible.

I think that adding a new face for this would be better, it makes custom
themes set it and change it dynamically; of course, the user can change
face attributes freely.

[0001-Add-ring-bell-function-for-flash-faces.-bug-77715.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

Message #44 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Fri, 11 Apr 2025 14:22:43 -0400

[Message part 1 (text/plain, inline)]

On Fri, Apr 11, 2025 at 2:18 PM Elijah Gabe Pérez <eg642616 <at> gmail.com>
wrote:

> Ship Mints <shipmints <at> gmail.com> writes:
> > I think the term duration is better than length when describing time
> intervals.
>
> I agree.
>
> > I still prefer the unwind-protected sit-for method that guarantees the
> remaps are removed to the one with
> > the timer where I've experienced errors intervening with a race
> condition creating new cookies that are
> > not removed.  Try mashing C-g with your timer implementation and you'll
> see it.
>
> The problem with using sit-for is that it delays the message displayed
> (usually "Quit") until timer stops (or if there is an input), it look
> like Emacs froze.
>

At 0.05 seconds, that's not likely.

Anyway, it changed it for use sit-for instead.
>

Cool.  You wrapped it with unwind-protect, yes?

> Is a new face really necessary just to implement a flash?  The
> implementation I proposed
> > allows the user to specify any face attributes to define what they want
> for a flash.  I wouldn't use this
> > feature without it being more reliable and flexible.
>
> I think that adding a new face for this would be better, it makes custom
> themes set it and change it dynamically; of course, the user can change
> face attributes freely.
>

As long as the implementation accepts a list of faces to flash as mine
does, that sounds good.  I'd just ignore the new face in my use.

[Message part 2 (text/html, inline)]

Message #47 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Ship Mints <shipmints <at> gmail.com>
Cc: 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Fri, 11 Apr 2025 12:47:40 -0600

Ship Mints <shipmints <at> gmail.com> writes:
>  Anyway, it changed it for use sit-for instead.
>
> Cool.  You wrapped it with unwind-protect, yes?

Yes.

>>> Is a new face really necessary just to implement a flash?  The implementation I proposed
>>> allows the user to specify any face attributes to define what they want for a flash.  I wouldn't use this
>>> feature without it being more reliable and flexible.
>>  I think that adding a new face for this would be better, it makes custom
>>  themes set it and change it dynamically; of course, the user can change
>>  face attributes freely.
> As long as the implementation accepts a list of faces to flash as mine does, that sounds good.  I'd just
> ignore the new face in my use.

Yeah it does:

+(defcustom flash-face-faces
+  '(mode-line-active)

By default it is set to `mode-line-active'.

-- 
                                          - E.G via GNU Emacs and Org.

Message #50 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: 77715 <at> debbugs.gnu.org
Cc: eliz <at> gnu.org, Ship Mints <shipmints <at> gmail.com>, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Sun, 27 Apr 2025 12:12:58 -0600

[Message part 1 (text/plain, inline)]

I don't know if there still interest on this,
I'm sending here a fixed and updated version of this patch.

This is ready to merge into master (unless there is something else).

[0001-New-ring-bell-functions-for-flash-faces.-bug-77715.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

Message #53 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 77715 <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Mon, 28 Apr 2025 14:35:32 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: Ship Mints <shipmints <at> gmail.com>, drew.adams <at> oracle.com, eliz <at> gnu.org
> Date: Sun, 27 Apr 2025 12:12:58 -0600
> 
> I don't know if there still interest on this,
> I'm sending here a fixed and updated version of this patch.
> 
> This is ready to merge into master (unless there is something else).

Thanks, but there are a few aspects of this that I think "need work".

> +** New face 'flash-face'.
> +This face is used for flash a face briefly.
> +Mainly used for 'flash-echo-area-bell-function' and
> +'flash-face-bell-function'.

I see no reason to announce this face in NEWS for the reasons I
explain below.

> +---
> +*** New function 'flash-face-bell-function'.
> +This function when called will flash a face briefly.
> +Intended to be used in 'ring-bell-function'.
> +
> +---
> +*** New function 'flash-echo-area-bell-function'.
> +This function when called will flash current echo area briefly.
> +Intended to be used in 'ring-bell-function'.

The "when called" part is redundant: it should be clear that a
function can do its job only if called.

> --- a/lisp/faces.el
> +++ b/lisp/faces.el
> @@ -2825,6 +2825,17 @@ header-line-inactive
>    :group 'mode-line-faces
>    :group 'basic-faces)
>  
> +(defface flash-face
> +  '((((class color) (min-colors 88))
> +     :background "red3" :foreground "white")
> +    (((class color grayscale) (min-colors 88))
> +     :background "grey10" :foreground "white")
> +    (t
> +     :inherit error :inverse-video t))
> +  "Basic face used for flash a face briefly."
> +  :version "31.1"
> +  :group 'basic-faces)

Can you tell why this face is defined in faces.el?  We don't need to
have it preloaded in Emacs.  Moreover, AFAIU this is not really a
face, to be used like other faces.  Instead, this is just a collection
of face attributes to add to other faces for producing the "flash"
effect.  So I'd rather we would not even use defface for it.  Or if
the alternatives are too awkward and inconvenient, at least let's make
the name of this face be an internal name (with two dashes), and
explain in the doc string how is this used.

> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -9389,6 +9389,50 @@ auto-save-mode
>    (and (< buffer-saved-size 0)
>         (setq buffer-saved-size 0)))

I don't think we need this in simple.el.  This is a minor convenience
feature, so it doesn't have to be preloaded.  It can be on a separate
file with the 2 functions it exports autoloaded.

> +(defcustom flash-face-duration 0.05
> +  "Time (in seconds) used for flash face duration."
> +  :type 'float
     ^^^^^^^^^^^^
Why not 'number instead?  Do you really want to support only floats?

> +(defcustom flash-face-faces
> +  '(mode-line-active)
> +  "A list of faces which `flash-face-bell-function' will flash them."

The doc string reads awkwardly.  I suggest

  Faces to be flashed by `flash-face-bell-function'.

> +(defun flash-face-bell-function ()
> +  "Flash a face or faces as ring a bell.

I'd say "Ring-bell function that flashes faces instead."

> +Intended to be used in `ring-bell-function'."
> +  (when-let* (flash-face-duration ; Ensure time is non-nil
                                       ^^^^^^^^^^^^^^^^^^^^^^
Why non-nil and not numberp?

> +              (cookies (mapcar (lambda (f) (face-remap-add-relative f 'flash-face))
> +                               flash-face-faces)))

Are you sure using face-remap-add-relative will work as expected with
any face in flash-face-faces?  Support for face remapping is not
magic, it must be explicitly coded in several places.  It might work
quite well for basic faces, but I'm not sure what happens with faces
defined by various packages.  For that reason, I'm not sure asking
users to specify a list of faces is the best idea.  How many different
faces did you test this with?  Did you try the default face, or
diff-mode faces, for example?

> +    (unwind-protect
> +        (sit-for flash-face-duration)
> +      (mapc #'face-remap-remove-relative cookies))))

Using sit-for means any input event will interrupt the wait.  Other
ring-bell-functions don't behave like that.  Are you sure sleep-for is
not better for this purpose?

> +(defun flash-echo-area-bell-function ()
> +  "Flash echo area as ring a bell.
> +Intended to be used in `ring-bell-function'."
> +  ;; like `with-temp-message' but suppress message log for not flood
> +  ;; messages buffer
> +  (unless (minibufferp) ; Avoid conflicts if cursor is in minibuffer

What conflict is that?

It seems like the above means the feature will not work (i.e., no
flash) if the minibuffer is active?  If so, is that a good idea?

> +    (let ((with-temp-message
> +              (propertize " " 'display `(space :align-to right) 'face
> +                          'flash-face))

Why do you use propertize here instead of face-remap-add-relative?

Message #56 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Elijah Gabe Pérez <eg642616 <at> gmail.com>,
 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Mon, 28 Apr 2025 09:06:42 -0400

[Message part 1 (text/plain, inline)]

On Mon, Apr 28, 2025 at 7:35 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > +              (cookies (mapcar (lambda (f) (face-remap-add-relative f
> 'flash-face))
> > +                               flash-face-faces)))
>
> Are you sure using face-remap-add-relative will work as expected with
> any face in flash-face-faces?  Support for face remapping is not
> magic, it must be explicitly coded in several places.  It might work
> quite well for basic faces, but I'm not sure what happens with faces
> defined by various packages.  For that reason, I'm not sure asking
> users to specify a list of faces is the best idea.  How many different
> faces did you test this with?  Did you try the default face, or
> diff-mode faces, for example?
>

I use my version of this implementation with the following list of faces:
'(internal-border tab-bar mode-line-active mode-line-inactive)

> +    (unwind-protect
> > +        (sit-for flash-face-duration)
> > +      (mapc #'face-remap-remove-relative cookies))))
>
> Using sit-for means any input event will interrupt the wait.  Other
> ring-bell-functions don't behave like that.  Are you sure sleep-for is
> not better for this purpose?
>

My implementation uses sit-for and the duration is short enough (mine is
set to 0.05 seconds) that this is fine.  And if it were any longer, I'd
want C-g to interrupt it and remove the remappings.

>
> > +(defun flash-echo-area-bell-function ()
> > +  "Flash echo area as ring a bell.
> > +Intended to be used in `ring-bell-function'."
> > +  ;; like `with-temp-message' but suppress message log for not flood
> > +  ;; messages buffer
> > +  (unless (minibufferp) ; Avoid conflicts if cursor is in minibuffer
>
> What conflict is that?
>
> It seems like the above means the feature will not work (i.e., no
> flash) if the minibuffer is active?  If so, is that a good idea?
>

My implementation doesn't have this condition so I'm not sure what it tries
to do or avoid.

-Stephane

[Message part 2 (text/html, inline)]

Message #59 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Mon, 28 Apr 2025 16:19:00 +0300

> From: Ship Mints <shipmints <at> gmail.com>
> Date: Mon, 28 Apr 2025 09:06:42 -0400
> Cc: Elijah Gabe Pérez <eg642616 <at> gmail.com>, 
> 	77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> 
> On Mon, Apr 28, 2025 at 7:35 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  > +              (cookies (mapcar (lambda (f) (face-remap-add-relative f 'flash-face))
>  > +                               flash-face-faces)))
> 
>  Are you sure using face-remap-add-relative will work as expected with
>  any face in flash-face-faces?  Support for face remapping is not
>  magic, it must be explicitly coded in several places.  It might work
>  quite well for basic faces, but I'm not sure what happens with faces
>  defined by various packages.  For that reason, I'm not sure asking
>  users to specify a list of faces is the best idea.  How many different
>  faces did you test this with?  Did you try the default face, or
>  diff-mode faces, for example?
> 
> I use my version of this implementation with the following list of faces: '(internal-border tab-bar
> mode-line-active mode-line-inactive)

These are all basic faces (see xfaces.c), for which the C code applies
face-remapping.  I asked about other faces.

>  > +    (unwind-protect
>  > +        (sit-for flash-face-duration)
>  > +      (mapc #'face-remap-remove-relative cookies))))
> 
>  Using sit-for means any input event will interrupt the wait.  Other
>  ring-bell-functions don't behave like that.  Are you sure sleep-for is
>  not better for this purpose?
> 
> My implementation uses sit-for and the duration is short enough (mine is set to 0.05 seconds) that this is
> fine.  And if it were any longer, I'd want C-g to interrupt it and remove the remappings.

What if the user want 0.1 sec, and there's an input event after just
10 msec?  The difference is perceptible by humans.

Message #62 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Mon, 28 Apr 2025 09:23:38 -0400

[Message part 1 (text/plain, inline)]

On Mon, Apr 28, 2025 at 9:19 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Ship Mints <shipmints <at> gmail.com>
> > Date: Mon, 28 Apr 2025 09:06:42 -0400
> > Cc: Elijah Gabe Pérez <eg642616 <at> gmail.com>,
> >       77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> >
> > On Mon, Apr 28, 2025 at 7:35 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> >  > +              (cookies (mapcar (lambda (f) (face-remap-add-relative
> f 'flash-face))
> >  > +                               flash-face-faces)))
> >
> >  Are you sure using face-remap-add-relative will work as expected with
> >  any face in flash-face-faces?  Support for face remapping is not
> >  magic, it must be explicitly coded in several places.  It might work
> >  quite well for basic faces, but I'm not sure what happens with faces
> >  defined by various packages.  For that reason, I'm not sure asking
> >  users to specify a list of faces is the best idea.  How many different
> >  faces did you test this with?  Did you try the default face, or
> >  diff-mode faces, for example?
> >
> > I use my version of this implementation with the following list of
> faces: '(internal-border tab-bar
> > mode-line-active mode-line-inactive)
>
> These are all basic faces (see xfaces.c), for which the C code applies
> face-remapping.  I asked about other faces.
>

I haven't tried others is what I was saying.  Elijah should test more faces
as part of the due diligence to get this into Emacs core.

>  > +    (unwind-protect
> >  > +        (sit-for flash-face-duration)
> >  > +      (mapc #'face-remap-remove-relative cookies))))
> >
> >  Using sit-for means any input event will interrupt the wait.  Other
> >  ring-bell-functions don't behave like that.  Are you sure sleep-for is
> >  not better for this purpose?
> >
> > My implementation uses sit-for and the duration is short enough (mine is
> set to 0.05 seconds) that this is
> > fine.  And if it were any longer, I'd want C-g to interrupt it and
> remove the remappings.
>
> What if the user want 0.1 sec, and there's an input event after just
> 10 msec?  The difference is perceptible by humans.
>

Same. The ring bell use case is sufficiently visually intrusive that if the
sit-for is interrupted by an input event, I'm good with that.

[Message part 2 (text/html, inline)]

Message #65 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 77715 <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Mon, 28 Apr 2025 13:04:53 -0600

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> --- a/lisp/faces.el
>> +++ b/lisp/faces.el
>> @@ -2825,6 +2825,17 @@ header-line-inactive
>>    :group 'mode-line-faces
>>    :group 'basic-faces)
>>  
>> +(defface flash-face
>> +  '((((class color) (min-colors 88))
>> +     :background "red3" :foreground "white")
>> +    (((class color grayscale) (min-colors 88))
>> +     :background "grey10" :foreground "white")
>> +    (t
>> +     :inherit error :inverse-video t))
>> +  "Basic face used for flash a face briefly."
>> +  :version "31.1"
>> +  :group 'basic-faces)
>
> Can you tell why this face is defined in faces.el?  We don't need to
> have it preloaded in Emacs.  Moreover, AFAIU this is not really a
> face, to be used like other faces.  Instead, this is just a collection
> of face attributes to add to other faces for producing the "flash"
> effect.  So I'd rather we would not even use defface for it.  Or if
> the alternatives are too awkward and inconvenient, at least let's make
> the name of this face be an internal name (with two dashes), and
> explain in the doc string how is this used.

I made it a defface specially for any custom theme that wants to set it.
So it can be "updated" to fit the current theme.

Anyway, I've changed it to a defcustom.

>> --- a/lisp/simple.el
>> +++ b/lisp/simple.el
>> @@ -9389,6 +9389,50 @@ auto-save-mode
>>    (and (< buffer-saved-size 0)
>>         (setq buffer-saved-size 0)))
>
> I don't think we need this in simple.el.  This is a minor convenience
> feature, so it doesn't have to be preloaded.  It can be on a separate
> file with the 2 functions it exports autoloaded.

Sure, but wouldn't be redundant making yet another file for just these 2
functions? I added to simple.el because i didn't know where to put the code.
In that case I have moved it to a separate file.

>> +(defcustom flash-face-duration 0.05
>> +  "Time (in seconds) used for flash face duration."
>> +  :type 'float
>      ^^^^^^^^^^^^
> Why not 'number instead?  Do you really want to support only floats?

Ops, i forgot it.

>> +Intended to be used in `ring-bell-function'."
>> +  (when-let* (flash-face-duration ; Ensure time is non-nil
>                                        ^^^^^^^^^^^^^^^^^^^^^^
> Why non-nil and not numberp?

Fixed.

>> +              (cookies (mapcar (lambda (f) (face-remap-add-relative f 'flash-face))
>> +                               flash-face-faces)))
>
> Are you sure using face-remap-add-relative will work as expected with
> any face in flash-face-faces?  Support for face remapping is not
> magic, it must be explicitly coded in several places.  It might work
> quite well for basic faces, but I'm not sure what happens with faces
> defined by various packages.  For that reason, I'm not sure asking
> users to specify a list of faces is the best idea.  How many different
> faces did you test this with?  Did you try the default face, or
> diff-mode faces, for example?

I've tested it with built-in faces and with some third-party packages;
it works fine:

  (setopt flash-face-faces
          '(mode-line-active
            mode-line
            
            default
            hl-line
            trailing-whitespace
            whitespace-trailing
            header-line-active
            header-line
            diff-added
            diff-removed
            diff-refine-added
            diff-indicator-added
            tab-bar
            tab-bar-tab
            centaur-tabs-selected
            centaur-tabs-close-selected
            solaire-mode-line-inactive-face
            solaire-mode-line-active-face))

And it should work for every face defined.

Asking to user to specify others faces is necessary from my POV,
there are packages that overrides some faces with their own one.

>> +    (unwind-protect
>> +        (sit-for flash-face-duration)
>> +      (mapc #'face-remap-remove-relative cookies))))
>
> Using sit-for means any input event will interrupt the wait.  Other
> ring-bell-functions don't behave like that.  Are you sure sleep-for is
> not better for this purpose?

Emacs would freeze if I use sleep-for instead, I've tested using it
but it seems that emacs is going to crash, very uncomfortable when I'm
editing quickly.  Using sit-for at least makes it noticeable and without
interrupting you while editing.  I had planned to use `run-with-timer'
but as Ship pointed, it is not recommended.

>> +(defun flash-echo-area-bell-function ()
>> +  "Flash echo area as ring a bell.
>> +Intended to be used in `ring-bell-function'."
>> +  ;; like `with-temp-message' but suppress message log for not flood
>> +  ;; messages buffer
>> +  (unless (minibufferp) ; Avoid conflicts if cursor is in minibuffer
>
> What conflict is that?
>
> It seems like the above means the feature will not work (i.e., no
> flash) if the minibuffer is active?  If so, is that a good idea?

When the cursor is the minibuffer (eval-expression, M-x and like) and the
ring-bell function is called the message is displayed as:
"[                                                                ]"
   ^ The bell face.

I found that it's caused by `minibuffer-message', but I'm not sure why
and how to remove the "[" "]".

I honestly think that while in the minibuffer this function is not
necessary since it already displays a message (since you are supposed to
be focused on the minibuffer)

For example:
 Typing `M-x' inside minibuffer displays message:
 [Command attempted to use minibuffer while in minibuffer]

 If this function (flash-echo-area-bell-function) is enabled there
 it would display:
 [
]

 and later:

 [Command attempted to use minibuffer while in minibuffer]


>> +    (let ((with-temp-message
>> +              (propertize " " 'display `(space :align-to right) 'face
>> +                          'flash-face))
>
> Why do you use propertize here instead of face-remap-add-relative?

That is for the echo-area/minibuffer, which AFAIK doesn't have a face
that i can use for this.  So instead it displays an empty message with
a face that extends to the end of minibuffer.

[0001-New-ring-bell-functions-for-flash-faces.-bug-77715.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

Message #68 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 77715 <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Tue, 29 Apr 2025 08:36:19 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 77715 <at> debbugs.gnu.org,  shipmints <at> gmail.com,  drew.adams <at> oracle.com
> Date: Mon, 28 Apr 2025 13:04:53 -0600
> 
> >> --- a/lisp/simple.el
> >> +++ b/lisp/simple.el
> >> @@ -9389,6 +9389,50 @@ auto-save-mode
> >>    (and (< buffer-saved-size 0)
> >>         (setq buffer-saved-size 0)))
> >
> > I don't think we need this in simple.el.  This is a minor convenience
> > feature, so it doesn't have to be preloaded.  It can be on a separate
> > file with the 2 functions it exports autoloaded.
> 
> Sure, but wouldn't be redundant making yet another file for just these 2
> functions?

Nothing wrong with having a separate file for this feature.  We could
call it ring-bell-fns.el or something, and with time other ring-bell
functions might be added to it.

Alternatively, we could add these two functions and the related
variables to subr-x.el.

> >> +    (unwind-protect
> >> +        (sit-for flash-face-duration)
> >> +      (mapc #'face-remap-remove-relative cookies))))
> >
> > Using sit-for means any input event will interrupt the wait.  Other
> > ring-bell-functions don't behave like that.  Are you sure sleep-for is
> > not better for this purpose?
> 
> Emacs would freeze if I use sleep-for instead, I've tested using it
> but it seems that emacs is going to crash, very uncomfortable when I'm
> editing quickly.

But that's how the built-in ring-bell functions work: they wait
unconditionally.  As I explained earlier, using sit-for might cause
the entire feature to seem not to work.

As a compromise, perhaps use sleep-for if the time is short enough,
and sit-for otherwise?

> >> +(defun flash-echo-area-bell-function ()
> >> +  "Flash echo area as ring a bell.
> >> +Intended to be used in `ring-bell-function'."
> >> +  ;; like `with-temp-message' but suppress message log for not flood
> >> +  ;; messages buffer
> >> +  (unless (minibufferp) ; Avoid conflicts if cursor is in minibuffer
> >
> > What conflict is that?
> >
> > It seems like the above means the feature will not work (i.e., no
> > flash) if the minibuffer is active?  If so, is that a good idea?
> 
> When the cursor is the minibuffer (eval-expression, M-x and like) and the
> ring-bell function is called the message is displayed as:
> "[                                                                ]"
>    ^ The bell face.
> 
> I found that it's caused by `minibuffer-message', but I'm not sure why
> and how to remove the "[" "]".

That's a feature: the "[...]" is intended to tell the user that the
message is from some background processing, unrelated to the
minibuffer prompt.  Why did you want to remove the brackets?

> I honestly think that while in the minibuffer this function is not
> necessary since it already displays a message (since you are supposed to
> be focused on the minibuffer)

Again, that's not how other ring-bell alternatives work.

> For example:
>  Typing `M-x' inside minibuffer displays message:
>  [Command attempted to use minibuffer while in minibuffer]
> 
>  If this function (flash-echo-area-bell-function) is enabled there
>  it would display:
>  [
> ]
> 
>  and later:
> 
>  [Command attempted to use minibuffer while in minibuffer]

This probably means there's some subtle bug in the current
implementation, and avoiding to flash when in the minibuffer just
sweeps the bug under the carpet.  I suggest to debug the reasons and
fix them, or at least understand them well enough so that we could
reason whether fixing them is a good idea.

> >> +    (let ((with-temp-message
> >> +              (propertize " " 'display `(space :align-to right) 'face
> >> +                          'flash-face))
> >
> > Why do you use propertize here instead of face-remap-add-relative?
> 
> That is for the echo-area/minibuffer, which AFAIK doesn't have a face
> that i can use for this.  So instead it displays an empty message with
> a face that extends to the end of minibuffer.

The face used by default in the echo-area is the default face.  (There
could be cases when echo-area messages use other, non-default faces.)

> +(defcustom flash-face-duration 0.05
> +  "Time (in seconds) used for flash face duration."
> +  :type 'number
> +  :group 'faces

Are you sure this is the best group for this option?  Why not
something related to flash-bell or bell-ring?  Same question regarding
the faces defined for this feature: placing them in the faces group
could drown them in the large sea of faces we have, and make them less
discoverable.

Message #71 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 77715 <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Tue, 29 Apr 2025 18:07:06 -0600

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
>> Cc: 77715 <at> debbugs.gnu.org,  shipmints <at> gmail.com,  drew.adams <at> oracle.com
>> Date: Mon, 28 Apr 2025 13:04:53 -0600
>> 
>> >> --- a/lisp/simple.el
>> >> +++ b/lisp/simple.el
>> >> @@ -9389,6 +9389,50 @@ auto-save-mode
>> >>    (and (< buffer-saved-size 0)
>> >>         (setq buffer-saved-size 0)))
>> >
>> > I don't think we need this in simple.el.  This is a minor convenience
>> > feature, so it doesn't have to be preloaded.  It can be on a separate
>> > file with the 2 functions it exports autoloaded.
>> 
>> Sure, but wouldn't be redundant making yet another file for just these 2
>> functions?
>
> Nothing wrong with having a separate file for this feature.  We could
> call it ring-bell-fns.el or something, and with time other ring-bell
> functions might be added to it.

It makes sense, I've renamed the file to `ring-bell-fns.el'.

>> >> +    (unwind-protect
>> >> +        (sit-for flash-face-duration)
>> >> +      (mapc #'face-remap-remove-relative cookies))))
>> >
>> > Using sit-for means any input event will interrupt the wait.  Other
>> > ring-bell-functions don't behave like that.  Are you sure sleep-for is
>> > not better for this purpose?
>> 
>> Emacs would freeze if I use sleep-for instead, I've tested using it
>> but it seems that emacs is going to crash, very uncomfortable when I'm
>> editing quickly.
>
> But that's how the built-in ring-bell functions work: they wait
> unconditionally.  As I explained earlier, using sit-for might cause
> the entire feature to seem not to work.
>
> As a compromise, perhaps use sleep-for if the time is short enough,
> and sit-for otherwise?

Fine, i've updated it to only use sleep-for if flash duration time is
long and fallback to sit-for otherwise.

>> >> +    (let ((with-temp-message
>> >> +              (propertize " " 'display `(space :align-to right) 'face
>> >> +                          'flash-face))
>> >
>> > Why do you use propertize here instead of face-remap-add-relative?
>> 
>> That is for the echo-area/minibuffer, which AFAIK doesn't have a face
>> that i can use for this.  So instead it displays an empty message with
>> a face that extends to the end of minibuffer.
>
> The face used by default in the echo-area is the default face.  (There
> could be cases when echo-area messages use other, non-default faces.)

Thanks for the clarification, I've rewrote the function to remap the
default face only in the minibuffer.

>> +(defcustom flash-face-duration 0.05
>> +  "Time (in seconds) used for flash face duration."
>> +  :type 'number
>> +  :group 'faces
>
> Are you sure this is the best group for this option?  Why not
> something related to flash-bell or bell-ring?  Same question regarding
> the faces defined for this feature: placing them in the faces group
> could drown them in the large sea of faces we have, and make them less
> discoverable.

I've create its proper defgroup, but I'm not sure about the name and
docstring I've given it.

[0001-New-ring-bell-functions-for-flash-faces.-bug-77715.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

Message #74 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 09:10:05 -0400

[Message part 1 (text/plain, inline)]

On Tue, Apr 29, 2025 at 8:07 PM Elijah Gabe Pérez <eg642616 <at> gmail.com>
wrote:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> >> Cc: 77715 <at> debbugs.gnu.org,  shipmints <at> gmail.com,  drew.adams <at> oracle.com
> >> Date: Mon, 28 Apr 2025 13:04:53 -0600
> >>
> >> >> --- a/lisp/simple.el
> >> >> +++ b/lisp/simple.el
> >> >> @@ -9389,6 +9389,50 @@ auto-save-mode
> >> >>    (and (< buffer-saved-size 0)
> >> >>         (setq buffer-saved-size 0)))
> >> >
> >> > I don't think we need this in simple.el.  This is a minor convenience
> >> > feature, so it doesn't have to be preloaded.  It can be on a separate
> >> > file with the 2 functions it exports autoloaded.
> >>
> >> Sure, but wouldn't be redundant making yet another file for just these 2
> >> functions?
> >
> > Nothing wrong with having a separate file for this feature.  We could
> > call it ring-bell-fns.el or something, and with time other ring-bell
> > functions might be added to it.
>
> It makes sense, I've renamed the file to `ring-bell-fns.el'.
>
> >> >> +    (unwind-protect
> >> >> +        (sit-for flash-face-duration)
> >> >> +      (mapc #'face-remap-remove-relative cookies))))
> >> >
> >> > Using sit-for means any input event will interrupt the wait.  Other
> >> > ring-bell-functions don't behave like that.  Are you sure sleep-for is
> >> > not better for this purpose?
> >>
> >> Emacs would freeze if I use sleep-for instead, I've tested using it
> >> but it seems that emacs is going to crash, very uncomfortable when I'm
> >> editing quickly.
> >
> > But that's how the built-in ring-bell functions work: they wait
> > unconditionally.  As I explained earlier, using sit-for might cause
> > the entire feature to seem not to work.
> >
> > As a compromise, perhaps use sleep-for if the time is short enough,
> > and sit-for otherwise?
>
> Fine, i've updated it to only use sleep-for if flash duration time is
> long and fallback to sit-for otherwise.
>
> >> >> +    (let ((with-temp-message
> >> >> +              (propertize " " 'display `(space :align-to right)
> 'face
> >> >> +                          'flash-face))
> >> >
> >> > Why do you use propertize here instead of face-remap-add-relative?
> >>
> >> That is for the echo-area/minibuffer, which AFAIK doesn't have a face
> >> that i can use for this.  So instead it displays an empty message with
> >> a face that extends to the end of minibuffer.
> >
> > The face used by default in the echo-area is the default face.  (There
> > could be cases when echo-area messages use other, non-default faces.)
>
> Thanks for the clarification, I've rewrote the function to remap the
> default face only in the minibuffer.
>
> >> +(defcustom flash-face-duration 0.05
> >> +  "Time (in seconds) used for flash face duration."
> >> +  :type 'number
> >> +  :group 'faces
> >
> > Are you sure this is the best group for this option?  Why not
> > something related to flash-bell or bell-ring?  Same question regarding
> > the faces defined for this feature: placing them in the faces group
> > could drown them in the large sea of faces we have, and make them less
> > discoverable.
>
> I've create its proper defgroup, but I'm not sure about the name and
> docstring I've given it.
>

Face flashing isn't a feature of bell ringing, it's the opposite.  I'd
consider putting the face flashing code with face-related code so it's
clear faces can be flashed anywhere for any reason the users want.

[Message part 2 (text/html, inline)]

Message #77 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 16:41:30 +0300

> From: Ship Mints <shipmints <at> gmail.com>
> Date: Wed, 30 Apr 2025 09:10:05 -0400
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> 
> Face flashing isn't a feature of bell ringing, it's the opposite.

??? Then why is ring-bell-functions being mentioned?

> I'd consider putting the face flashing code with
> face-related code so it's clear faces can be flashed anywhere for any reason the users want.

But this code is not general enough for that.  It was written
explicitly for additional optional values for ring-bell-functions, as
the doc strings say.

Message #80 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 09:52:44 -0400

[Message part 1 (text/plain, inline)]

On Wed, Apr 30, 2025 at 9:41 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Ship Mints <shipmints <at> gmail.com>
> > Date: Wed, 30 Apr 2025 09:10:05 -0400
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 77715 <at> debbugs.gnu.org,
> drew.adams <at> oracle.com
> >
> > Face flashing isn't a feature of bell ringing, it's the opposite.
>
> ??? Then why is ring-bell-functions being mentioned?
>
> > I'd consider putting the face flashing code with
> > face-related code so it's clear faces can be flashed anywhere for any
> reason the users want.
>
> But this code is not general enough for that.  It was written
> explicitly for additional optional values for ring-bell-functions, as
> the doc strings say.
>

The prototype was derived from private code used in this narrow case but if
we're going to adopt the functionality in core, I think we go the extra
mile to make it appropriately general.

[Message part 2 (text/html, inline)]

Message #83 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 17:06:22 +0300

> From: Ship Mints <shipmints <at> gmail.com>
> Date: Wed, 30 Apr 2025 09:52:44 -0400
> Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> 
> On Wed, Apr 30, 2025 at 9:41 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  > From: Ship Mints <shipmints <at> gmail.com>
>  > Date: Wed, 30 Apr 2025 09:10:05 -0400
>  > Cc: Eli Zaretskii <eliz <at> gnu.org>, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
>  > 
>  > Face flashing isn't a feature of bell ringing, it's the opposite.
> 
>  ??? Then why is ring-bell-functions being mentioned?
> 
>  > I'd consider putting the face flashing code with
>  > face-related code so it's clear faces can be flashed anywhere for any reason the users want.
> 
>  But this code is not general enough for that.  It was written
>  explicitly for additional optional values for ring-bell-functions, as
>  the doc strings say.
> 
> The prototype was derived from private code used in this narrow case but if we're going to adopt the
> functionality in core, I think we go the extra mile to make it appropriately general.

But then the entire implementation should be revisited and reviewed
with that generality in mind.  So please let's talk about that.  Could
you or someone else please describe what general features are meant to
be implemented based on this functionality?

Message #86 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 10:09:36 -0400

[Message part 1 (text/plain, inline)]

On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Ship Mints <shipmints <at> gmail.com>
> > Date: Wed, 30 Apr 2025 09:52:44 -0400
> > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> >
> > On Wed, Apr 30, 2025 at 9:41 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> >  > From: Ship Mints <shipmints <at> gmail.com>
> >  > Date: Wed, 30 Apr 2025 09:10:05 -0400
> >  > Cc: Eli Zaretskii <eliz <at> gnu.org>, 77715 <at> debbugs.gnu.org,
> drew.adams <at> oracle.com
> >  >
> >  > Face flashing isn't a feature of bell ringing, it's the opposite.
> >
> >  ??? Then why is ring-bell-functions being mentioned?
> >
> >  > I'd consider putting the face flashing code with
> >  > face-related code so it's clear faces can be flashed anywhere for any
> reason the users want.
> >
> >  But this code is not general enough for that.  It was written
> >  explicitly for additional optional values for ring-bell-functions, as
> >  the doc strings say.
> >
> > The prototype was derived from private code used in this narrow case but
> if we're going to adopt the
> > functionality in core, I think we go the extra mile to make it
> appropriately general.
>
> But then the entire implementation should be revisited and reviewed
> with that generality in mind.  So please let's talk about that.  Could
> you or someone else please describe what general features are meant to
> be implemented based on this functionality?
>

I think of face flashing as the face equivalent to '
pulse-momentary-highlight-region'.

[Message part 2 (text/html, inline)]

Message #89 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 17:29:14 +0300

> From: Ship Mints <shipmints <at> gmail.com>
> Date: Wed, 30 Apr 2025 10:09:36 -0400
> Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> 
> On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  >  But this code is not general enough for that.  It was written
>  >  explicitly for additional optional values for ring-bell-functions, as
>  >  the doc strings say.
>  > 
>  > The prototype was derived from private code used in this narrow case but if we're going to adopt the
>  > functionality in core, I think we go the extra mile to make it appropriately general.
> 
>  But then the entire implementation should be revisited and reviewed
>  with that generality in mind.  So please let's talk about that.  Could
>  you or someone else please describe what general features are meant to
>  be implemented based on this functionality?
> 
> I think of face flashing as the face equivalent to 'pulse-momentary-highlight-region'.

Then why don't we use the functions defined in pulse.el in the first
place? why invent a whole new family of functions,l face attributes,
etc.?

Message #92 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 10:33:52 -0400

[Message part 1 (text/plain, inline)]

On Wed, Apr 30, 2025 at 10:30 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Ship Mints <shipmints <at> gmail.com>
> > Date: Wed, 30 Apr 2025 10:09:36 -0400
> > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> >
> > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> >  >  But this code is not general enough for that.  It was written
> >  >  explicitly for additional optional values for ring-bell-functions, as
> >  >  the doc strings say.
> >  >
> >  > The prototype was derived from private code used in this narrow case
> but if we're going to adopt the
> >  > functionality in core, I think we go the extra mile to make it
> appropriately general.
> >
> >  But then the entire implementation should be revisited and reviewed
> >  with that generality in mind.  So please let's talk about that.  Could
> >  you or someone else please describe what general features are meant to
> >  be implemented based on this functionality?
> >
> > I think of face flashing as the face equivalent to
> 'pulse-momentary-highlight-region'.
>
> Then why don't we use the functions defined in pulse.el in the first
> place? why invent a whole new family of functions,l face attributes,
> etc.?
>

Overlays require a buffer and flashing faces like the inner border or
tab-bar can't be accomplished using pulse.el functions unless I'm missing
something.

[Message part 2 (text/html, inline)]

Message #95 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 17:52:18 +0300

> From: Ship Mints <shipmints <at> gmail.com>
> Date: Wed, 30 Apr 2025 10:33:52 -0400
> Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> 
> On Wed, Apr 30, 2025 at 10:30 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  > From: Ship Mints <shipmints <at> gmail.com>
>  > Date: Wed, 30 Apr 2025 10:09:36 -0400
>  > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
>  > 
>  > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>  > 
>  >  >  But this code is not general enough for that.  It was written
>  >  >  explicitly for additional optional values for ring-bell-functions, as
>  >  >  the doc strings say.
>  >  > 
>  >  > The prototype was derived from private code used in this narrow case but if we're going to adopt
>  the
>  >  > functionality in core, I think we go the extra mile to make it appropriately general.
>  > 
>  >  But then the entire implementation should be revisited and reviewed
>  >  with that generality in mind.  So please let's talk about that.  Could
>  >  you or someone else please describe what general features are meant to
>  >  be implemented based on this functionality?
>  > 
>  > I think of face flashing as the face equivalent to 'pulse-momentary-highlight-region'.
> 
>  Then why don't we use the functions defined in pulse.el in the first
>  place? why invent a whole new family of functions,l face attributes,
>  etc.?
> 
> Overlays require a buffer and flashing faces like the inner border or tab-bar can't be accomplished using
> pulse.el functions unless I'm missing something.

But the code as submitted included echo-area as well, where there _is_
a buffer.

And my question about the faces is still unanswered.

Message #98 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Ship Mints <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 10:55:02 -0400

[Message part 1 (text/plain, inline)]

On Wed, Apr 30, 2025 at 10:52 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Ship Mints <shipmints <at> gmail.com>
> > Date: Wed, 30 Apr 2025 10:33:52 -0400
> > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> >
> > On Wed, Apr 30, 2025 at 10:30 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> >  > From: Ship Mints <shipmints <at> gmail.com>
> >  > Date: Wed, 30 Apr 2025 10:09:36 -0400
> >  > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> >  >
> >  > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >  >
> >  >  >  But this code is not general enough for that.  It was written
> >  >  >  explicitly for additional optional values for
> ring-bell-functions, as
> >  >  >  the doc strings say.
> >  >  >
> >  >  > The prototype was derived from private code used in this narrow
> case but if we're going to adopt
> >  the
> >  >  > functionality in core, I think we go the extra mile to make it
> appropriately general.
> >  >
> >  >  But then the entire implementation should be revisited and reviewed
> >  >  with that generality in mind.  So please let's talk about that.
> Could
> >  >  you or someone else please describe what general features are meant
> to
> >  >  be implemented based on this functionality?
> >  >
> >  > I think of face flashing as the face equivalent to
> 'pulse-momentary-highlight-region'.
> >
> >  Then why don't we use the functions defined in pulse.el in the first
> >  place? why invent a whole new family of functions,l face attributes,
> >  etc.?
> >
> > Overlays require a buffer and flashing faces like the inner border or
> tab-bar can't be accomplished using
> > pulse.el functions unless I'm missing something.
>
> But the code as submitted included echo-area as well, where there _is_
> a buffer.
>

My implementation did not.  Elijah needs to defend that.

And my question about the faces is still unanswered.
>

That's for Elijah to explain, that was his choice.

I prefer the simplest possible thing that works and my original that I
shared seems optimal and is only tangentially related to bell ringing
because that's how I use it ATM.

[Message part 2 (text/html, inline)]

Message #101 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 18:17:28 +0300

> From: Ship Mints <shipmints <at> gmail.com>
> Date: Wed, 30 Apr 2025 10:55:02 -0400
> Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> 
> On Wed, Apr 30, 2025 at 10:52 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  > From: Ship Mints <shipmints <at> gmail.com>
>  > Date: Wed, 30 Apr 2025 10:33:52 -0400
>  > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
>  > 
>  > On Wed, Apr 30, 2025 at 10:30 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>  > 
>  >  > From: Ship Mints <shipmints <at> gmail.com>
>  >  > Date: Wed, 30 Apr 2025 10:09:36 -0400
>  >  > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
>  >  > 
>  >  > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>  >  > 
>  >  >  >  But this code is not general enough for that.  It was written
>  >  >  >  explicitly for additional optional values for ring-bell-functions, as
>  >  >  >  the doc strings say.
>  >  >  > 
>  >  >  > The prototype was derived from private code used in this narrow case but if we're going to
>  adopt
>  >  the
>  >  >  > functionality in core, I think we go the extra mile to make it appropriately general.
>  >  > 
>  >  >  But then the entire implementation should be revisited and reviewed
>  >  >  with that generality in mind.  So please let's talk about that.  Could
>  >  >  you or someone else please describe what general features are meant to
>  >  >  be implemented based on this functionality?
>  >  > 
>  >  > I think of face flashing as the face equivalent to 'pulse-momentary-highlight-region'.
>  > 
>  >  Then why don't we use the functions defined in pulse.el in the first
>  >  place? why invent a whole new family of functions,l face attributes,
>  >  etc.?
>  > 
>  > Overlays require a buffer and flashing faces like the inner border or tab-bar can't be accomplished
>  using
>  > pulse.el functions unless I'm missing something.
> 
>  But the code as submitted included echo-area as well, where there _is_
>  a buffer.
> 
> My implementation did not.  Elijah needs to defend that.
> 
>  And my question about the faces is still unanswered.
> 
> That's for Elijah to explain, that was his choice.

then perhaps Elijah had different plans for this?  E.g., maybe he
didn't intend to generalize this code at all?

So let's wait for Elijah to chime in, okay?

Message #104 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 77715 <at> debbugs.gnu.org, Ship Mints <shipmints <at> gmail.com>,
 drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Wed, 30 Apr 2025 11:51:46 -0600

Eli Zaretskii <eliz <at> gnu.org> wrote:
> > From: Ship Mints <shipmints <at> gmail.com>
> > Date: Wed, 30 Apr 2025 10:09:36 -0400
> > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> > 
> > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > 
> >  >  But this code is not general enough for that.  It was written
> >  >  explicitly for additional optional values for ring-bell-functions, as
> >  >  the doc strings say.
> >  > 
> >  > The prototype was derived from private code used in this narrow case but 
> > if we're going to adopt the
> >  > functionality in core, I think we go the extra mile to make it 
> > appropriately general.
> > 
> >  But then the entire implementation should be revisited and reviewed
> >  with that generality in mind.  So please let's talk about that.  Could
> >  you or someone else please describe what general features are meant to
> >  be implemented based on this functionality?
> > 
> > I think of face flashing as the face equivalent to 
> > 'pulse-momentary-highlight-region'.

> Then why don't we use the functions defined in pulse.el in the first
> place? why invent a whole new family of functions,l face attributes,
> etc.?

pulse.el is somewhat complex for me; as you pointed, the ring bell must
wait until `sleep-for' ends, pulse.el uses `run-with-timer' which will
not stop/freeze Emacs.

I wasn't sure how to reuse pulse.el code for that
(and I'm not sure if it is slow than this implementation).

And the echo-area function will ensure all the echo-area/minibuffer
background face will flash; pulse.el will only flash a region in the
minibuffer instead the whole buffer face.

The code is intended to flash/pulse `any face'.

If it is required to move this code for general purposes.
I'm not sure where to move it, maybe to pulse.el?

But of course, the ring-bell functions must be kept but be rewritten for
use that feature.

-- 
                                          - E.G via GNU Emacs and Org.

Message #107 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 77715 <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Thu, 01 May 2025 20:11:36 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: Ship Mints <shipmints <at> gmail.com>,  77715 <at> debbugs.gnu.org,
>   drew.adams <at> oracle.com
> Date: Wed, 30 Apr 2025 11:51:46 -0600
> 
> Eli Zaretskii <eliz <at> gnu.org> wrote:
> > > From: Ship Mints <shipmints <at> gmail.com>
> > > Date: Wed, 30 Apr 2025 10:09:36 -0400
> > > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
> > > 
> > > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > > 
> > >  >  But this code is not general enough for that.  It was written
> > >  >  explicitly for additional optional values for ring-bell-functions, as
> > >  >  the doc strings say.
> > >  > 
> > >  > The prototype was derived from private code used in this narrow case but 
> > > if we're going to adopt the
> > >  > functionality in core, I think we go the extra mile to make it 
> > > appropriately general.
> > > 
> > >  But then the entire implementation should be revisited and reviewed
> > >  with that generality in mind.  So please let's talk about that.  Could
> > >  you or someone else please describe what general features are meant to
> > >  be implemented based on this functionality?
> > > 
> > > I think of face flashing as the face equivalent to 
> > > 'pulse-momentary-highlight-region'.
> 
> > Then why don't we use the functions defined in pulse.el in the first
> > place? why invent a whole new family of functions,l face attributes,
> > etc.?
> 
> pulse.el is somewhat complex for me; as you pointed, the ring bell must
> wait until `sleep-for' ends, pulse.el uses `run-with-timer' which will
> not stop/freeze Emacs.
> 
> I wasn't sure how to reuse pulse.el code for that
> (and I'm not sure if it is slow than this implementation).
> 
> And the echo-area function will ensure all the echo-area/minibuffer
> background face will flash; pulse.el will only flash a region in the
> minibuffer instead the whole buffer face.
> 
> The code is intended to flash/pulse `any face'.
> 
> If it is required to move this code for general purposes.
> I'm not sure where to move it, maybe to pulse.el?
> 
> But of course, the ring-bell functions must be kept but be rewritten for
> use that feature.

What future directions do you see for extending and generalizing this
functionality?  In particular, do you think it is basically limited to
ring-bell-functions and similar, or are you planning on extending this
for other cases, and if so, what are the plans?

Message #110 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 77715 <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Thu, 01 May 2025 21:50:22 -0600

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
>> Cc: Ship Mints <shipmints <at> gmail.com>,  77715 <at> debbugs.gnu.org,
>>   drew.adams <at> oracle.com
>> Date: Wed, 30 Apr 2025 11:51:46 -0600
>> 
>> Eli Zaretskii <eliz <at> gnu.org> wrote:
>> > > From: Ship Mints <shipmints <at> gmail.com>
>> > > Date: Wed, 30 Apr 2025 10:09:36 -0400
>> > > Cc: eg642616 <at> gmail.com, 77715 <at> debbugs.gnu.org, drew.adams <at> oracle.com
>> > > 
>> > > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>> > > 
>> > >  >  But this code is not general enough for that.  It was written
>> > >  >  explicitly for additional optional values for ring-bell-functions, as
>> > >  >  the doc strings say.
>> > >  > 
>> > >  > The prototype was derived from private code used in this narrow case but 
>> > > if we're going to adopt the
>> > >  > functionality in core, I think we go the extra mile to make it 
>> > > appropriately general.
>> > > 
>> > >  But then the entire implementation should be revisited and reviewed
>> > >  with that generality in mind.  So please let's talk about that.  Could
>> > >  you or someone else please describe what general features are meant to
>> > >  be implemented based on this functionality?
>> > > 
>> > > I think of face flashing as the face equivalent to 
>> > > 'pulse-momentary-highlight-region'.
>> 
>> > Then why don't we use the functions defined in pulse.el in the first
>> > place? why invent a whole new family of functions,l face attributes,
>> > etc.?
>> 
>> pulse.el is somewhat complex for me; as you pointed, the ring bell must
>> wait until `sleep-for' ends, pulse.el uses `run-with-timer' which will
>> not stop/freeze Emacs.
>> 
>> I wasn't sure how to reuse pulse.el code for that
>> (and I'm not sure if it is slow than this implementation).
>> 
>> And the echo-area function will ensure all the echo-area/minibuffer
>> background face will flash; pulse.el will only flash a region in the
>> minibuffer instead the whole buffer face.
>> 
>> The code is intended to flash/pulse `any face'.
>> 
>> If it is required to move this code for general purposes.
>> I'm not sure where to move it, maybe to pulse.el?
>> 
>> But of course, the ring-bell functions must be kept but be rewritten for
>> use that feature.

> What future directions do you see for extending and generalizing this
> functionality?

Well, this feature can be implemented in pulse.el, but in separated
functions (since pulse.el functions only works for text/regions).
So this can be useful for packages that want to flash/pulse briefly a
face for indicate something.

Sadly, this feature doesn't use the `pulse smooth effect' unless
I rewrite (or duplicate) most pulse.el code for that, so for now it just
"pulse" without the fancy effect.

> In particular, do you think it is basically limited to
> ring-bell-functions and similar, or are you planning on extending this
> for other cases?

Well, the original code wasn't not limited to ring-bell-functions, it
was a kind of alternative to pulse.el, but since this fits better there,
I've moved and rewrote it to pulse.el, so anyone can use this for
anything.

Also I've updated the ring-bell functions for use that.

[0001-New-pulse-functions-for-pulse-faces-and-new-file-for.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

Message #113 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 77715 <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Sat, 10 May 2025 13:01:07 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 77715 <at> debbugs.gnu.org,  shipmints <at> gmail.com,  drew.adams <at> oracle.com
> Date: Tue, 29 Apr 2025 18:07:06 -0600

Thanks, a few comments related to English grammar and wording in the
documentation part of this patch:

> +---
> +*** New function 'flash-face-bell-function'.
> +This function flash a face briefly.
                 ^^^^^
"flashes"

> +Intended to be used in 'ring-bell-function'.
   ^^^^^^^^
"It is intended"

> +---
> +*** New function 'flash-echo-area-bell-function'.
> +This function flash current echo area briefly.
> +Intended to be used in 'ring-bell-function'.

Same here.

> +---
> +*** New user option 'flash-face-duration'.
> +This option controls flash duration for 'flash-face-bell-function' and
                        ^^^^^^^^^^^^^^
"the flash duration"

> +*** New user option 'flash-face-attributes'
> +This option tells 'flash-face-bell-function' and
> +'flash-echo-area-bell-function' which face attributes should use
> +for flash.                                            ^^^^^^^^^^

"should be used"

> +This is intended to be used in any function from `ring-bell-fns' such as
> +`flash-face-bell-function' and `flash-echo-area-bell-function' for make
> +the flash face more noticeable."                               ^^^^^^^^

"to make"

> +(defun flash-echo-area-bell-function ()
> +  "Flash echo area as ring a bell.

Suggest to rephrase:

  "Indicate ringing the bell by flashing the echo area."

Message #116 received at 77715 <at> debbugs.gnu.org (full text, mbox):

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 77715 <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Sat, 10 May 2025 18:39:57 -0600

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
>> Cc: 77715 <at> debbugs.gnu.org,  shipmints <at> gmail.com,  drew.adams <at> oracle.com
>> Date: Tue, 29 Apr 2025 18:07:06 -0600
>
> Thanks, a few comments related to English grammar and wording in the
> documentation part of this patch:
>
>> +---
>> +*** New function 'flash-face-bell-function'.
>> +This function flash a face briefly.
>                  ^^^^^
[...]

Thanks, I've updated the patch.

[0001-New-pulse-functions-for-pulse-faces-and-new-file-for.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

Message #121 received at 77715-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 77715-done <at> debbugs.gnu.org, shipmints <at> gmail.com, drew.adams <at> oracle.com
Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and
 header line.
Date: Sat, 17 May 2025 11:58:41 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 77715 <at> debbugs.gnu.org,  shipmints <at> gmail.com,  drew.adams <at> oracle.com
> Date: Sat, 10 May 2025 18:39:57 -0600
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> >> Cc: 77715 <at> debbugs.gnu.org,  shipmints <at> gmail.com,  drew.adams <at> oracle.com
> >> Date: Tue, 29 Apr 2025 18:07:06 -0600
> >
> > Thanks, a few comments related to English grammar and wording in the
> > documentation part of this patch:
> >
> >> +---
> >> +*** New function 'flash-face-bell-function'.
> >> +This function flash a face briefly.
> >                  ^^^^^
> [...]
> 
> Thanks, I've updated the patch.

Thanks, now installed on the master branch, and closing the bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.