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

From: Eli Zaretskii <[email-masked]>
Date: Mon, 28 Apr 2025 16:19:00 +0300

> From: Ship Mints <[email-masked]>
> Date: Mon, 28 Apr 2025 09:06:42 -0400
> Cc: Elijah Gabe Pérez <[email-masked]>, 
> 	[email-masked], [email-masked]
> 
> On Mon, Apr 28, 2025 at 7:35 AM Eli Zaretskii <[email-masked]> 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)

These are all basic faces (see xfaces.c), for which the C code applies
face-remapping.  I asked about other faces.

>  > +    (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.

What if the user want 0.1 sec, and there's an input event after just
10 msec?  The difference is perceptible by humans.