> From: Ship Mints <shipmints@gmail.com>
> Date: Mon, 28 Apr 2025 09:06:42 -0400
> Cc: Elijah Gabe Pérez <eg642616@gmail.com>,
> 77715@debbugs.gnu.org, drew.adams@oracle.com
>
> On Mon, Apr 28, 2025 at 7:35 AM Eli Zaretskii <eliz@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)
These are all basic faces (see xfaces.c), for which the C code applies
face-remapping. I asked about other faces.
I haven't tried others is what I was saying. Elijah should test more faces as part of the due diligence to get this into Emacs core.
> > + (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.
Same. The ring bell use case is sufficiently visually intrusive that if the sit-for is interrupted by an input event, I'm good with that.