GNU bug report logs -
#77715
[PATCH] Add ring-bell functions for mode line and header line.
Previous Next
Full log
Message #68 received at 77715 <at> debbugs.gnu.org (full text, mbox):
> 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 56 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.