unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: "Kévin Le Gouguec" <kevin.legouguec@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: gregory@heytings.org, 60841@debbugs.gnu.org
Subject: bug#60841: 30.0.50; kill-ring-save pauses despite region being highlighted
Date: Mon, 23 Jan 2023 23:29:00 +0100	[thread overview]
Message-ID: <87lelsga1f.fsf@gmail.com> (raw)
In-Reply-To: <833581jtff.fsf@gnu.org> (Eli Zaretskii's message of "Mon, 23 Jan 2023 15:01:56 +0200")

Eli Zaretskii <eliz@gnu.org> 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).





  reply	other threads:[~2023-01-23 22:29 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-15 23:38 bug#60841: 30.0.50; kill-ring-save pauses despite region being highlighted Kévin Le Gouguec
2023-01-16 12:47 ` Eli Zaretskii
2023-01-16 21:58   ` Kévin Le Gouguec
2023-01-16 22:28   ` Gregory Heytings
2023-01-17  7:53     ` Kévin Le Gouguec
2023-01-17  8:26       ` Gregory Heytings
2023-01-17 22:03         ` Kévin Le Gouguec
2023-01-18 13:12           ` Eli Zaretskii
2023-01-18 22:16             ` Kévin Le Gouguec
2023-01-21  8:08               ` Eli Zaretskii
2023-01-22 22:45                 ` Kévin Le Gouguec
2023-01-23 13:01                   ` Eli Zaretskii
2023-01-23 22:29                     ` Kévin Le Gouguec [this message]
2023-01-24 13:23                       ` Eli Zaretskii
2023-01-28 17:45                         ` Kévin Le Gouguec
2023-01-28 18:07                           ` Eli Zaretskii
2023-01-29 14:54                             ` Kévin Le Gouguec
2023-01-29 15:40                               ` Eli Zaretskii
2023-01-29 22:57                                 ` Kévin Le Gouguec
2023-01-30 12:41                                   ` Eli Zaretskii
2023-01-30 22:38                                     ` Kévin Le Gouguec
2023-02-02 10:43                                       ` Eli Zaretskii
2023-02-02 21:15                                         ` Kévin Le Gouguec
2023-01-29 17:55                               ` Juri Linkov
2023-01-29 19:09                                 ` Eli Zaretskii
2023-01-29 19:33                                   ` Eli Zaretskii
2023-01-29 20:32                                     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lelsga1f.fsf@gmail.com \
    --to=kevin.legouguec@gmail.com \
    --cc=60841@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=gregory@heytings.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).