emacs-29 c4988840598: Avoid spurious pause in kill-ring-save (Bug#60841)

[Kévin Le Gouguec]

branch: emacs-29
commit c4988840598b7da84b25d21a1936ce1ab6f6d666
Author: Kévin Le Gouguec <kevin.legouguec@gmail.com>
Commit: Kévin Le Gouguec <kevin.legouguec@gmail.com>

    Avoid spurious pause in kill-ring-save (Bug#60841)
    
    'indicate-copied-region' checks whether the region is "highlighted"
    and if not, briefly moves point to mark to give a visual cue of the
    extent of text that was saved to the kill ring.
    
    The region is considered "highlighted" if (a) it is active and (b) its
    face specifies a :background.  That latter condition does not account
    for the multiple ways in which the face can make the region "visually
    distinct" from the default face, so switch to the more extensive
    predicate face-differs-from-default-p.
    
    The patch also fixes a couple of issues with the predicate's
    implementation, and introduces a new user option in case anyone
    happened to enjoy unconditional blinking.
    
    * lisp/faces.el (face-differs-from-default-p): Filter out :extend; add
    rationale for the attributes we ignore.
    * lisp/simple.el (copy-region-blink-predicate): Add option to let
    users explicitly opt into or out of blinking point and mark.
    (region-indistinguishable-p): New function to detect
    "if there is currently no active region highlighting", leveraging
    face-differs-from-default-p.
    (indicate-copied-region): Use it.
    * src/xfaces.c (merge_face_ref): Allow :stipple to be nil, since it is
    a documented valid value for that attribute.
    * etc/NEWS: Announce user option.
---
 etc/NEWS       | 12 ++++++++++++
 lisp/faces.el  | 11 ++++++++++-
 lisp/simple.el | 22 ++++++++++++++++++++--
 src/xfaces.c   |  3 +--
 4 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d402cc82c4a..e3cbeb84d30 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1353,6 +1353,18 @@ dragged.
 Customize this option to limit the number of entries in the menu
 "Edit → Paste from Kill Menu".  The default is 60.
 
+---
+** New user option 'copy-region-blink-predicate'.
+By default, when copying a region with 'kill-ring-save', Emacs only
+blinks point and mark when the region is not denoted visually, that
+is, when either the region is inactive, or the 'region' face is
+indistinguishable from the 'default' face.
+
+Users who would rather enable blinking unconditionally can now set
+this user option to 'always'.  To disable blinking unconditionally,
+either set this option to 'ignore', or set 'copy-region-blink-delay'
+to 0.
+
 +++
 ** Performing a pinch gesture on a touchpad now increases the text scale.
 
diff --git a/lisp/faces.el b/lisp/faces.el
index 3323eab205a..4933b495a6c 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -304,7 +304,16 @@ If the optional argument FRAME is given, report on face 
FACE in that frame.
 If FRAME is t, report on the defaults for face FACE (for new frames).
 If FRAME is omitted or nil, use the selected frame."
   (let ((attrs
-        (delq :inherit (mapcar 'car face-attribute-name-alist)))
+         ;; The _value_ of :inherit teaches us nothing about how FACE
+         ;; looks compared to the default face.  Instead, we will ask
+         ;; `face-attribute' to take inheritance into account when
+         ;; examining other attributes.
+         (delq :inherit
+               ;; A difference in extension past EOL only matters when
+               ;; relevant attributes (such as :background) also
+               ;; differ from the default; otherwise this difference
+               ;; is a false positive.
+               (delq :extend (mapcar 'car face-attribute-name-alist))))
        (differs nil))
     (while (and attrs (not differs))
       (let* ((attr (pop attrs))
diff --git a/lisp/simple.el b/lisp/simple.el
index 861fe193fb8..c58acfe3adc 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5852,6 +5852,25 @@ The value 0 disables blinking."
   :group 'killing
   :version "28.1")
 
+(defcustom copy-region-blink-predicate #'region-indistinguishable-p
+  "Whether the cursor must be blinked after a copy.
+When this condition holds, and the copied region fits in the
+current window, `kill-ring-save' will blink the cursor between
+point and mark for `copy-region-blink-delay' seconds."
+  :type '(radio (function-item region-indistinguishable-p)
+                (function-item :doc "Always blink point and mark." always)
+                (function-item :doc "Never blink point and mark." ignore)
+                (function :tag "Other predicate function"))
+  :group 'killing
+  :version "29.1")
+
+(defun region-indistinguishable-p ()
+  "Whether the current region is not denoted visually.
+This holds when the region is inactive, or when the `region' face
+cannot be distinguished from the `default' face."
+  (not (and (region-active-p)
+            (face-differs-from-default-p 'region))))
+
 (defun indicate-copied-region (&optional message-len)
   "Indicate that the region text has been copied interactively.
 If the mark is visible in the selected window, blink the cursor between
@@ -5872,8 +5891,7 @@ of this sample text; it defaults to 40."
        ;; 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))
-                      (not (face-background 'region nil t))))
+                  (funcall copy-region-blink-predicate))
          ;; Swap point and mark.
          (set-marker (mark-marker) (point) (current-buffer))
          (goto-char mark)
diff --git a/src/xfaces.c b/src/xfaces.c
index 35b79154805..62d7823f308 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -2780,8 +2780,7 @@ merge_face_ref (struct window *w,
              else if (EQ (keyword, QCstipple))
                {
 #if defined (HAVE_WINDOW_SYSTEM)
-                 Lisp_Object pixmap_p = Fbitmap_spec_p (value);
-                 if (!NILP (pixmap_p))
+                 if (NILP (value) || !NILP (Fbitmap_spec_p (value)))
                    to[LFACE_STIPPLE_INDEX] = value;
                  else
                    err = true;