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

From: Elijah Gabe Pérez <[email-masked]>
Date: Tue, 29 Apr 2025 18:07:06 -0600

Eli Zaretskii <[email-masked]> writes:

>> From: Elijah Gabe Pérez <[email-masked]>
>> Cc: [email-masked],  [email-masked],  [email-masked]
>> 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.

>From 9ed6088f05a54ef4e6a974ad157dff2d749e9b8c 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 ring-bell functions for flash faces.  (bug#77715)

New functions for flash any face or echo area once whether
`ring-bell-function' is called.
* lisp/ring-bell-fns.el: New file.
* etc/NEWS: Announce changes.
---
 etc/NEWS              | 25 ++++++++++++
 lisp/ring-bell-fns.el | 95 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)
 create mode 100644 lisp/ring-bell-fns.el

diff --git a/etc/NEWS b/etc/NEWS
index a5936b1c6cf..6eee690e09b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1935,6 +1935,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/ring-bell-fns.el b/lisp/ring-bell-fns.el
new file mode 100644
index 00000000000..e9ab59ef34f
--- /dev/null
+++ b/lisp/ring-bell-fns.el
@@ -0,0 +1,95 @@
+;;; 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 'face-remap)
+
+(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-duration 0.05
+  "Time (in seconds) used for flash face duration."
+  :type 'number
+  :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'."
+  (when-let* (((numberp flash-face-duration)) ; Ensure time is a number
+              (cookies (mapcar (lambda (f) (apply #'face-remap-add-relative f flash-face-attributes))
+                               flash-face-faces)))
+    (unwind-protect
+        ;; Use sit-for if the duration is very long for not blocking
+        ;; emacs, otherwise fallback to sleep-for.
+        (if (> flash-face-duration 0.1)
+            (sit-for flash-face-duration)
+          ;; And redisplay for apply the face remap
+          (redisplay)
+          (sleep-for flash-face-duration))
+      (mapc #'face-remap-remove-relative cookies))))
+
+;;;###autoload
+(defun flash-echo-area-bell-function ()
+  "Flash echo area as ring a bell.
+Intended to be used in `ring-bell-function'."
+  (with-current-buffer (window-buffer (minibuffer-window)) ; Get in minibuffer buffer
+    (let ((cookie (apply #'face-remap-add-relative 'default flash-face-attributes)))
+      ;; When minibuffer is not displaying anything (empty) the face
+      ;; remap will not work, insert an temporal space in the
+      ;; minibuffer ONLY if it's empty for fix this.
+      (if (= (buffer-size) 0)
+          (insert ?\s))
+      (unwind-protect
+          ;; Use sit-for if the duration is very long for not blocking
+          ;; emacs, otherwise fallback to sleep-for.
+          (if (> flash-face-duration 0.1)
+              (sit-for flash-face-duration)
+            ;; And redisplay for apply the face remap
+            (redisplay)
+            (sleep-for flash-face-duration))
+        (face-remap-remove-relative cookie)))))
+
+(provide 'ring-bell-fns)
+;;; ring-bell-fns.el ends here
-- 
2.49.0

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