Subject: Re: bug#77715: [PATCH] Add ring-bell functions for mode line and header line.

From: Elijah Gabe Pérez <[email-masked]>
Date: Thu, 01 May 2025 21:50:22 -0600

Eli Zaretskii <[email-masked]> writes:

>> From: Elijah Gabe Pérez <[email-masked]>
>> Cc: Ship Mints <[email-masked]>,  [email-masked],
>>   [email-masked]
>> Date: Wed, 30 Apr 2025 11:51:46 -0600
>> 
>> Eli Zaretskii <[email-masked]> wrote:
>> > > From: Ship Mints <[email-masked]>
>> > > Date: Wed, 30 Apr 2025 10:09:36 -0400
>> > > Cc: [email-masked], [email-masked], [email-masked]
>> > > 
>> > > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii <[email-masked]> 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.

>From 498f0a869edbc003b239fe3cce1eb660b81ab510 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?El=C3=ADas=20Gabriel=20P=C3=A9rez?= <[email-masked]>
Date: Thu, 10 Apr 2025 11:38:21 -0600
Subject: [PATCH] New pulse functions for pulse faces and new file for ring
 bell fns.

These new pulse functions allow pulse any defined face briefly.
The new file contains functions intended to be used in
`ring-bell-function' as alternatives to `visible-bell'

* etc/NEWS: Announce changes
* lisp/pulse.el (pulse-face-duration): New user option.
(pulse-flash-face): New function.
* lisp/ring-bell-fns.el (flash-face-attributes)
(flash-face-faces): New user options.
(flash-face-bell-function, flash-echo-area-bell-function):
New functions.  (bug#77715)
---
 etc/NEWS              | 25 ++++++++++++++
 lisp/pulse.el         | 42 +++++++++++++++++++++++
 lisp/ring-bell-fns.el | 80 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 147 insertions(+)
 create mode 100644 lisp/ring-bell-fns.el

diff --git a/etc/NEWS b/etc/NEWS
index d149a7387e2..ef1a636e887 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1943,6 +1943,31 @@ change their face if the current line exceeds the 'fill-column'.  The
 new face 'display-fill-column-indicator-warning-face' is used to
 highlight the fill-column indicators.  By default, this is disabled.
 
+---
+*** New function 'flash-face-bell-function'.
+This function flash a face briefly.
+Intended to be used in 'ring-bell-function'.
+
+---
+*** New function 'flash-echo-area-bell-function'.
+This function flash current echo area briefly.
+Intended to be used in 'ring-bell-function'.
+
+---
+*** New user option 'flash-face-duration'.
+This option controls flash duration for 'flash-face-bell-function' and
+'flash-echo-area-bell-function'.
+
+---
+*** New user option 'flash-face-faces'.
+This option tells 'flash-face-bell-function' which faces should flash.
+
+---
+*** 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.
+
 ---
 ** Flymake
 
diff --git a/lisp/pulse.el b/lisp/pulse.el
index a2569338e11..c91521c7d5b 100644
--- a/lisp/pulse.el
+++ b/lisp/pulse.el
@@ -227,6 +227,48 @@ pulse-line-hook-function
   (when pulse-command-advice-flag
     (pulse-momentary-highlight-one-line (point))))
 
+;;; Pulse faces
+;; Functions for pulse any defined face.
+(require 'face-remap)
+
+(defcustom pulse-face-duration pulse-delay
+  "Time (in seconds) used for pulse face duration."
+  :type 'number
+  :group 'pulse
+  :version "31.1")
+
+;; FIXME: The pulse smooth effect cannot be archieved here due
+;;        the face remaping will not work well for that.
+(defun pulse-faces (faces &optional with-face)
+  "Pulse FACES with face WITH-FACE (if defined) briefly.
+FACES must be a list of faces to pulse.
+WITH-FACE is optional, it can be a defined face or a list of face
+properties to apply."
+  (when-let* (((numberp pulse-face-duration)) ; Ensure time is a number
+              (with-face (or with-face 'pulse-highlight-face))
+              (in-buffer (current-buffer))
+              (cookies (mapcar (lambda (f)
+                                 (if (consp with-face)
+                                     (apply #'face-remap-add-relative f with-face)
+                                   (face-remap-add-relative f with-face)))
+                               faces)))
+    ;; Use run-with-timer if the duration is very long for not blocking
+    ;; emacs, otherwise fallback to sleep-for.
+    (if (> pulse-face-duration 0.1)
+        (run-with-timer pulse-face-duration 0
+                        (lambda ()
+                          ;; Remove the face remaping in the buffer
+                          ;; where `pulse-faces' was called.
+                          (if (buffer-live-p in-buffer)
+                              (with-current-buffer in-buffer
+                                (mapc #'face-remap-remove-relative cookies)))))
+      (unwind-protect
+          ;; redisplay for apply the face remap
+          (progn
+            (redisplay)
+            (sleep-for pulse-face-duration))
+        (mapc #'face-remap-remove-relative cookies)))))
+
 (provide 'pulse)
 
 ;;; pulse.el ends here
diff --git a/lisp/ring-bell-fns.el b/lisp/ring-bell-fns.el
new file mode 100644
index 00000000000..ce947496657
--- /dev/null
+++ b/lisp/ring-bell-fns.el
@@ -0,0 +1,80 @@
+;;; ring-bell-fns.el --- Collection of functions for ring-bell  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2025 Free Software Foundation, Inc.
+
+;; Author: Elijah Gabe Pérez <[email-masked]>
+;; Keywords: faces
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Collection of functions intended to be used with `ring-bell-function'.
+;; as alternatives to `visible-bell'
+
+;;; Code:
+(require 'pulse)
+
+(defgroup ring-bell nil
+  "Customization options for ring bell."
+  :version "31.1"
+  :group 'emacs)
+
+(defcustom flash-face-attributes
+  '(:background "red" :foreground "white")
+  "Face attributes to use in any function from `ring-bell-fns'.
+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."
+  :type 'plist
+  :version "31.1")
+
+(defcustom flash-face-faces
+  '(mode-line-active)
+  "A list of faces to be flashed by `flash-face-bell-function'."
+  :type '(repeat face)
+  :version "31.1")
+
+;;;###autoload
+(defun flash-face-bell-function ()
+  "Ring-bell function that flashes faces.
+Intended to be used in `ring-bell-function'."
+  (pulse-faces flash-face-faces flash-face-attributes))
+
+(defun flash--echo-area-bell-1 (buf)
+  (with-current-buffer buf ; Get in minibuffer/echo area buffer
+    (pulse-faces '(default) flash-face-attributes)))
+
+;;;###autoload
+(defun flash-echo-area-bell-function ()
+  "Flash minibuffer or echo area as ring a bell.
+Intended to be used in `ring-bell-function'."
+  ;; pulse-faces uses run-with-timer if `pulse-face-duration'
+  ;; is long, which makes the flashing in the echo area not visible.
+  ;; for fix this then apply the flashing to *Echo Area 0*
+  ;; and minibuffer buffer for the `run-with-timer',
+  ;; and fallback to minibuffer buffer if otherwise due
+  ;; performance.
+  (if (> pulse-face-duration 0.1)
+      (dolist (buf `(,(window-buffer (minibuffer-window))
+                     ;; get or create the echo area for flash it too.
+                     ,(get-buffer-create" *Echo Area 0*")))
+        (redisplay)
+        (flash--echo-area-bell-1 buf))
+    (flash--echo-area-bell-1 (window-buffer (minibuffer-window)))))
+
+(provide 'ring-bell-fns)
+;;; ring-bell-fns.el ends here
-- 
2.49.0

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