Eli Zaretskii writes: >> From: Elijah Gabe Pérez >> Cc: Ship Mints , 77715@debbugs.gnu.org, >> drew.adams@oracle.com >> Date: Wed, 30 Apr 2025 11:51:46 -0600 >> >> Eli Zaretskii wrote: >> > > From: Ship Mints >> > > Date: Wed, 30 Apr 2025 10:09:36 -0400 >> > > Cc: eg642616@gmail.com, 77715@debbugs.gnu.org, drew.adams@oracle.com >> > > >> > > On Wed, Apr 30, 2025 at 10:06 AM Eli Zaretskii wrote: >> > > >> > > > But this code is not general enough for that. It was written >> > > > explicitly for additional optional values for ring-bell-functions, as >> > > > the doc strings say. >> > > > >> > > > The prototype was derived from private code used in this narrow case but >> > > if we're going to adopt the >> > > > functionality in core, I think we go the extra mile to make it >> > > appropriately general. >> > > >> > > But then the entire implementation should be revisited and reviewed >> > > with that generality in mind. So please let's talk about that. Could >> > > you or someone else please describe what general features are meant to >> > > be implemented based on this functionality? >> > > >> > > I think of face flashing as the face equivalent to >> > > 'pulse-momentary-highlight-region'. >> >> > Then why don't we use the functions defined in pulse.el in the first >> > place? why invent a whole new family of functions,l face attributes, >> > etc.? >> >> pulse.el is somewhat complex for me; as you pointed, the ring bell must >> wait until `sleep-for' ends, pulse.el uses `run-with-timer' which will >> not stop/freeze Emacs. >> >> I wasn't sure how to reuse pulse.el code for that >> (and I'm not sure if it is slow than this implementation). >> >> And the echo-area function will ensure all the echo-area/minibuffer >> background face will flash; pulse.el will only flash a region in the >> minibuffer instead the whole buffer face. >> >> The code is intended to flash/pulse `any face'. >> >> If it is required to move this code for general purposes. >> I'm not sure where to move it, maybe to pulse.el? >> >> But of course, the ring-bell functions must be kept but be rewritten for >> use that feature. > What future directions do you see for extending and generalizing this > functionality? Well, this feature can be implemented in pulse.el, but in separated functions (since pulse.el functions only works for text/regions). So this can be useful for packages that want to flash/pulse briefly a face for indicate something. Sadly, this feature doesn't use the `pulse smooth effect' unless I rewrite (or duplicate) most pulse.el code for that, so for now it just "pulse" without the fancy effect. > In particular, do you think it is basically limited to > ring-bell-functions and similar, or are you planning on extending this > for other cases? Well, the original code wasn't not limited to ring-bell-functions, it was a kind of alternative to pulse.el, but since this fits better there, I've moved and rewrote it to pulse.el, so anyone can use this for anything. Also I've updated the ring-bell functions for use that.