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

Previous Next

Full log

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)]

Previous Next

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