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

[Kévin Le Gouguec]

Gregory Heytings <gregory@heytings.org> writes:

>>>> But it would be better to have there a test which would tell us whether the
>>>> region face is "visually different" from the default face. Can we do
>>>> something like that?
>>>
>>> face-differs-from-default-p?
>>
>> Nice catch; IIUC display-supports-face-attributes-p is what is doing the 
>> heavy
>> work.
>>
>> Might need to be made smarter wrt :extend though?
>>
>
> Indeed...
>
>>
>> (set-face-background 'region (face-background 'default nil t))
>> (face-differs-from-default-p 'region nil)
>> ; ⇒ :extend
>>
>
> ... although that's a really contrived example.

OK, how about this example:

>   (defun mtmm-reset-background ()
>     (set-face-attribute 'region nil :background 'unspecified))

Straight from <87vfjhre9z.fsf@zamazal.org> (emacs-devel), i.e. the user
report which resulted in the current code in indicate-copied-region.

Same result: :extend differs, so (face-differs-from-default-p 'region)
returns t, which for this user would cause a regression compared to
(face-background 'region nil t).

> I'm not sure it's TRT to add too much complexity there, so I would suggest to
> add an additional optional argument to face-differs-from-default-p, to exclude
> one or more attributes from the comparison (in this case :extend, but someone
> else might be interested in excluding other attributes in the future).

I really think how face-differs-from-default-p examines attributes…

              (and
               (not (eq attr-val 'unspecified))
               (display-supports-face-attributes-p (list attr attr-val)
                                                   frame))

… makes no sense for :extend.  Here's an example (and this one is
contrived alright):

  (set-face-attribute 'default nil :underline t)
  (set-face-attribute 'region nil :underline nil :extend VALUE)

* VALUE is 'unspecified: the condition becomes nil (meaning ":extend
  does not make this face look different than default"), whereas that
  actually _does_ make a difference (:underline nil too, obviously, and
  luckily face-differs-from-default-p picks up on this),

* VALUE is nil: (display-supports-face-attributes-p '(:extend nil))
  returns nil (because see gui_supports_face_attributes_p), despite
  :extend nil procuding different results for :underline.

* VALUE is t: for the same reason, display-supports-face-attributes-p
  returns t, despite :extend t making :underline indistinguishable from
  the default face.

Granted, yeah, contrived, since that calls for :underline t on the
default face and who does that.

But since, as we've seen, :extend causes a false positive (face reported
as "displaying differently" despite displaying identically) when a face
has a :background equal to the default's, I don't see how the current
behaviour helps anyone.