GNU bug report logs -
#77715
[PATCH] Add ring-bell functions for mode line and header line.
Previous Next
Full log
View this message in rfc822 format
> 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.
This bug report was last modified 3 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.