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

From: Kévin Le Gouguec <[email-masked]>
Date: Mon, 23 Jan 2023 23:29:00 +0100

Eli Zaretskii <[email-masked]> writes:

>> * that seq-difference call makes me inexplicably nervous; I thought I
>>   vaguely remembered debates on whether preloaded libraries {c,sh}ould
>>   use seq.el functions, but then I see that "emacs-lisp/seq" is already
>>   in preloaded-file-list, and e.g. rmc.el calls some of its functions.
>>   Am I misremembering?
>
> seq.el is indeed preloaded, so that ship has sailed.  But you still
> need to make sure seq is loaded _before_ any preloaded file which uses
> it, and in this case faces is loaded before seq, so you cannot use
> seq-difference.

(Thanks for spelling this out.  Do we have any documentation that calls
out the precautions one must take when writing Elisp that will be
preloaded, or any tooling that can detect whether some of those
precautions were forgotten?  FWIW I saw no compiler warnings nor runtime
errors with that patch)

> And, btw, I cannot say I understand why using seq-difference here is
> justified.  Why not just call delq twice and be done?

That would work, of course; will go with that for the next revision.

(No justification beyond lack of practice at reading or writing
"preloaded Elisp", so my brain takes a couple more cycles to translate
(delq 'a (delq 'b x)) into "x \ {a,b}")

>> >> (Hm, and against my better judgement I went ahead and compared
>> >> gui_supports_face_attributes_p vs tty_supports_face_attributes_p, and I
>> >> see that they handle :extend differently and *mashes C-c C-c with
>> >> forehead before fingers can type another wall of text*)
>> >
>> > TTY frames always extend the color, that's the reason for the
>> > difference.
>> 
>> (Not sure I get your meaning here; on the Linux TTY I have on hand,
>> (set-face-extend 'region nil) does disable color extension)
>
> I'm sorry, you will have to look up the discussion that led to the
> development of the :extend attribute; I cannot afford searching for
> it.

(Did I mention I'm grateful for the time you and Gregory did afford for
this obscure issue, considering how busy this rc phase is?)

>      The differences between TTY and GUI frames were one of the main
> reasons why we introduced this attribute.  Maybe what I remember
> happens only on some terminals.  Or maybe I'm misremembering and it
> was because of underline and not the color.  But there is definitely a
> difference.

ACK, will look into this.

>> +(defun region-highlighted-p ()
>> +  "Say whether the region is visibly highlighted.
>
> Please drop the "Say" part, it's not our style.

ACK.  I see a few matches for "Return whether…" in-tree; would…

  Return whether the region stands out visually.

… be OK, or should I just go for…

  Return non-nil if the region stands out visually.

?

(Asking because I have a (tiny, subduable) preference for the former;
predicate docstrings that call out nil/t/non-nil force me to pause and
figure out whether I need to negate the rest of the sentence, whereas
"whether" generally precedes a description of the "true" case.

"(elisp) Documentation Tips" recommends "Return t if", but merely as a
way to "avoid starting the sentence with “t”", not because we have a
preference for literally starting with "Return t if")

> And I'd use some other word instead of "highlighted", because that
> could be misinterpreted (the region is supposed to be highlighted).
> Something like "stands-out" perhaps?

Agreed.  stands-out works for me (other candidates that come to mind:
noticeable, conspicuous).