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

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> 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.
>
> IMNSHO, this just makes another mini-step (albeit in the right
> direction), instead of going all the way.  There are other face
> attributes that can make the region stand out on display: what about
> underline or italics, for example?  And vice versa: the face could
> have a background that is identical or almost identical to the default
> face.

Indeed.  I wasn't sure how much brain-racking we would feel like
devoting to this issue, hence the naive patch; thank you for prompting
more discussion.

> We could keep adding conditions for more and more face attributes, one
> by one.  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?

I struggle to find a heuristic that I'd dare call "smart", so here's a
dumb one: if _any_ attribute of the region face differs from that of the
default face, then odds are whoever customized the region face (theme
author or end user) intended for it to "stand out" to some extent?

(defun dumb/stands-out-from-default-p ()
  (seq-some
   (lambda (attribute)
     (unless (memq attribute '(:extend :inherit))
       (let ((default-value (face-attribute 'default attribute nil t))
             (region-value (face-attribute 'region attribute nil t)))
         (and (not (eq region-value 'unspecified))
              (not (equal region-value default-value))))))
   (mapcar 'car (face-all-attributes 'default))))

> Alternatively, we could add a user option to make the swap
> unconditional, because maybe some users would prefer that to splitting
> hair in this case.  Then we could stop worrying about all those fine
> differences.

Right, after discovering the feature swap I was surprised to find no way
to enable it regardless of whether the region is highlighted.  Sure,
OT1H I thought Emacs was bugging out on me; OTOH now knowing what's
happening (and in particular, knowing that we're in sit-for so not
frozen), considering that the region is deactivated when M-w ends, I
could imagine some people finding this swap helpful and preferring to
have it on always.


Wrapping up…

(a) Would it make sense to introduce a defcustom
    (copy-region-indication-predicate?) that allows nil, t, or a
    predicate function?  The default value being:

    (and (region-active-p)
         (smart/stands-out-from-default-p))

    (Random considerations:
     * nil would be redundant with copy-region-blink-delay 0 or nil,
       not sure how much we care,
     * we could require this to always be a function instead, since
       'always is a thing)

(b) No flash of genius on how to determine whether the region face
    "stands out", yet.