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

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>>     ▼ Copy Region Inhibit Blink:
>>     Choice:
>>     (*) region-stands-out-p
>>             Whether the region can be distinguished visually. More
>>     ( ) always
>>             Always inhibit: never blink point and mark.
>>     ( ) ignore
>>             Never inhibit: always blink point and mark.
>>     ( ) Other predicate function.: ignore
>>         State : STANDARD.
>>        Whether we should refrain from blinking the cursor after a copy. ▼
>>        When this condition holds, ‘kill-ring-save’ will not blink the
>>        cursor between point and mark to denote the copied region.
>>     Groups: Killing
>> 
>>                                  ⁂ VS ⁂
>> 
>>     ▼ Copy Region Blink Predicate:
>>     Choice:
>>     (*) 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.
>>     ( ) always
>>             Always blink point and mark.
>>     ( ) ignore
>>             Never blink point and mark.
>>     ( ) Other predicate function.: ignore
                                   ^
                                   (Booh, that's ugly.  Removed in the
                                    attached)
>>         State : STANDARD.
>>        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.
>>     Groups: Killing
>
> I prefer the second one, since inhibit-SOMETHING is slightly harder to
> grasp, due to the negation.

ACK.

> The second paragraph of the NEWS entry shouldn't be there: it tells
> that we fixed a bug.

Right; in this instance I wondered if we should call out what some users
could perceive as a "regression".  I suppose it doesn't matter; if they
are indeed reading NEWS, they'll find this entry whether it mentions the
bug or not (since it mentions kill-ring-save).

Adjusted the entry in the attached.

Re. :stipple:

> The problem with :stipple is that we don't allow nil as the value of
> :stipple.  If you evaluate
> 
>   (setq list-matching-lines-prefix-face 'default)
>   (face-differs-from-default-p list-matching-lines-prefix-face)
> 
> then look in *Messages*, you will see:
> 
>   Invalid face attribute :stipple nil

(Œil de lynx !)

> So we need to treat :stipple specially, or maybe fix merge_face_ref to
> allow the nil value.

You mean something like the following?  (Thoroughly untested, beside
passing the (null (face-differs-from-default-p 'default)) test)

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;


OT1H that's the kind of sleeping dragon that I wouldn't want to tickle,
OTOH I see that other spots in the code seem to accept QCstipple mapping
to a NILP value:

    else if (EQ (attr, QCstipple))
      {
  #if defined (HAVE_WINDOW_SYSTEM)
        if (!UNSPECIFIEDP (value)
            && !IGNORE_DEFFACE_P (value)
            && !RESET_P (value)
            && !NILP (value)
            && NILP (Fbitmap_spec_p (value)))
          signal_error ("Invalid stipple attribute", value);
        old_value = LFACE_STIPPLE (lface);
        ASET (lface, LFACE_STIPPLE_INDEX, value);
  #endif /* HAVE_WINDOW_SYSTEM */
      }
— Finternal_set_lisp_face_attribute

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