bug#60841: 30.0.50; kill-ring-save pauses despite region being highlighted

[Kévin Le Gouguec]

Hello Emacs,

kill-ring-save calls indicate-copied-region, which does a quick
point-and-mark swap (for copy-region-blink-delay seconds) to give the
user an idea of the extent of text that has been saved.

The docstring says that this is done "if there is currently no active
region highlighting"; in practice, this translates to:

        ;; Swap point-and-mark quickly so as to show the region that
        ;; was selected.  Don't do it if the region is highlighted.
        (when (and (numberp copy-region-blink-delay)
                   (> copy-region-blink-delay 0)
                   (or (not (region-active-p))                  ; (a)
                       (not (face-background 'region nil t))))  ; (b)

IOW "active region highlighting" means "(a) an active region, and (b)
any background for the region face".  face-background, however, is not
enough to asses (b); consider, from emacs -Q:

M-: (custom-set-faces '(region ((t (:foreground "blue" :inverse-video t)))))
C-x h
M-w

In this situation, the region face has a clearly visible background, yet
indicate-copied-region still behaves as if the region is not
"highlighted", and triggers the (harmless and entirely interruptible)
point-and-mark swap followed by a pause.

I did not know about this feature, and only found out about it by
accident after customizing the region face.  Only after (1) assuming
something was off between Emacs and the system clipboard (2) failing to
use debug-on-quit to figure out where Emacs was "stuck" (it wasn't, of
course) (3) diving into kill-ring-save, did I realize what was going on.

The attached patch handles this foreground + inverse-video switcheroo.
Not sure how many themes actually do that in the wild, so I'd understand
if there wasn't much enthusiasm for applying.  That face-background
check has been with us since 2004; haven't found any hint in the BTS
that anyone else has been bothered by this false negative.


(FWIW, I stumbled on this while working on a dark theme with a limited
 palette; having exhausted all backgrounds on other faces and struggling
 to find one to dedicate to the region, I figured I could combine one of
 my bright foreground colors with :inverse-video, and leave :background
 unspecified.

 Of course, instead of:

   :foreground SOMETHING-BRIGHT
   :inverse-video t

 I could specify:

   :background SOMETHING-BRIGHT
   :foreground SOMETHING-DARK

 … but my initial idea was to let the chips fall where they may and
 allow any text background to become a foreground within the region,
 since all of the theme's backgrounds ought to be dark enough to
 contrast with SOMETHING-BRIGHT)


Thanks for your time.


 GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.35, cairo version 1.17.6) of 2023-01-02 built on amdahl30
Repository revision: c209802f7b3721a1b95113290934a23fee88f678
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101006
System Description: openSUSE Tumbleweed

Configured using:
 'configure --with-cairo --with-gconf --with-sqlite3 --with-xinput2'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix

0001-Avoid-spurious-pause-in-kill-ring-save.patch
Description: Text Data