From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?K=C3=A9vin?= Le Gouguec Newsgroups: gmane.emacs.bugs Subject: bug#60841: 30.0.50; kill-ring-save pauses despite region being highlighted Date: Sun, 22 Jan 2023 23:45:23 +0100 Message-ID: <87fsc2qjcs.fsf@gmail.com> References: <87h6wrs71h.fsf@gmail.com> <83zgai4peg.fsf@gnu.org> <5583fd58387746ce7ddc@heytings.org> <87cz7dbns0.fsf@gmail.com> <4c2c6cf44ad37e405b06@heytings.org> <878ri0g6ob.fsf@gmail.com> <83pmbc0yxo.fsf@gnu.org> <87y1pzo5dp.fsf@gmail.com> <834jskmhs8.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="19338"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: gregory@heytings.org, 60841@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Jan 22 23:46:16 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pJj6N-0004oH-8F for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 22 Jan 2023 23:46:15 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pJj6E-0005d4-EN; Sun, 22 Jan 2023 17:46:06 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pJj6A-0005cn-Ek for bug-gnu-emacs@gnu.org; Sun, 22 Jan 2023 17:46:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pJj6A-0004uI-6Z for bug-gnu-emacs@gnu.org; Sun, 22 Jan 2023 17:46:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pJj69-0003cf-Lm for bug-gnu-emacs@gnu.org; Sun, 22 Jan 2023 17:46:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?K=C3=A9vin?= Le Gouguec Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 22 Jan 2023 22:46:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 60841 X-GNU-PR-Package: emacs Original-Received: via spool by 60841-submit@debbugs.gnu.org id=B60841.167442753713886 (code B ref 60841); Sun, 22 Jan 2023 22:46:01 +0000 Original-Received: (at 60841) by debbugs.gnu.org; 22 Jan 2023 22:45:37 +0000 Original-Received: from localhost ([127.0.0.1]:52781 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pJj5k-0003bu-Em for submit@debbugs.gnu.org; Sun, 22 Jan 2023 17:45:37 -0500 Original-Received: from mail-wm1-f49.google.com ([209.85.128.49]:40804) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pJj5f-0003bb-Kb for 60841@debbugs.gnu.org; Sun, 22 Jan 2023 17:45:35 -0500 Original-Received: by mail-wm1-f49.google.com with SMTP id e19-20020a05600c439300b003db1cac0c1fso7928842wmn.5 for <60841@debbugs.gnu.org>; Sun, 22 Jan 2023 14:45:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=7bpPSPgt3H6kWVVAE0L1Ucduvyn1vf1feVTTAa+DZhQ=; b=hMMnV2bt6xJpdGnBYa/x/QyTEt3JNm4eYUuh+kQie4LbbYzQo71vc14KE8xJOnak4H lgZPj4U9zGIVPA62ozFAf3tNhP2Q9JtSVfbL2IGt4tVdJCI6w9s31zJgvckN+2r2zkHR +Xa1X7aV8Z37hz594oe7eZDkRBEyhpTheeh03Y+Qwa629IzAJSy3+yOAjAYKVh14pqNz HZohFJZm6D9A4sDzuj3x/Jx0uB3009oTFXNyUE9yrGjMhgNC1siwUvruubWrz99nE06j CNTCuBGkwNPJhkPLCTFaz/dyFp4/doUesHGnkyZo6IxrsBVfuvTMAKVhjrjnrzCIq2Cf MkEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=7bpPSPgt3H6kWVVAE0L1Ucduvyn1vf1feVTTAa+DZhQ=; b=GCi3FDGwKMASIKnwbeiORwShwC9bU7Jmo7zuLBDQfbNjUWxJV0MGNKE4XV3wND5WHa C4rtOljFGsox39RuKdZdX/n5ONHHP0/+G/1YfBRmO69tIPxRxE0hyFLL+PAPvKYYzbOS nKIEcQfQVTiQVrKcwvSwueH7OsRboLufVIh9KX6XbOMkoM/5ZBZwkM4Z8jUIihUL1QtU C69LNmDSRSonP5gR0lAQKM1DyQBECYeGbt/D+qebr8gYyS2DkwF+C4z7bBoJ5ekVeqgZ gv8uz2/m+h1sS2pk5KWl58XVAXZbflXRh6lJaMTJ51jluGXqz3XL6S9CZr1ALJ2t9mw7 /FVw== X-Gm-Message-State: AFqh2krKdwWftgWS7o8boK2udba6jBpU0CcIGVlfPrHL8Cf2SP0F2Kin 8NFWwpRvmNjreUUbDXRUVmRUSZYv/Cw= X-Google-Smtp-Source: AMrXdXsYtA9Ort5XviAmq+k4hetx+FlOn1ZvEbZ4rccOo9j6qI/+tX2jjGO8UZwMowiVvomXXKxhYw== X-Received: by 2002:a05:600c:1d0f:b0:3db:9ed:aaf8 with SMTP id l15-20020a05600c1d0f00b003db09edaaf8mr20335591wms.33.1674427525231; Sun, 22 Jan 2023 14:45:25 -0800 (PST) Original-Received: from amdahl30 ([2a01:e0a:253:fe0:2ef0:5dff:fed2:7b49]) by smtp.gmail.com with ESMTPSA id i8-20020a05600c4b0800b003da2932bde0sm10230421wmp.23.2023.01.22.14.45.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 22 Jan 2023 14:45:24 -0800 (PST) In-Reply-To: <834jskmhs8.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 21 Jan 2023 10:08:23 +0200") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:253967 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Eli Zaretskii writes: >> From: K=C3=A9vin Le Gouguec >> Cc: gregory@heytings.org, 60841@debbugs.gnu.org >> Date: Wed, 18 Jan 2023 23:16:34 +0100 >>=20 >> > * :extend nil for both: they display differently (region will not be >> > extended, default will be), >> > * :extend t for both: they display the same, >> > * default has nil, region has t: they display the same, >> > * default has t, region has nil: they display differently. > > The default face is always extended, so it should be treated as > implicitly having the :extend attribute set. > > I think face-differs-from-default-p should be fixed to either ignore > the :extend attribute like it ignores :inherit (since it could be > argued that :extend doesn't really control how the face affects > characters on display), or it should treat the default face as having > the :extend attribute with the value t. I have been slowly converging toward "ignore :extend" being TRT. I cannot find a situation where :extend matters. AFAICT "does this face stand out vs 'default?" always comes down to whether the face's :underline or :background _also_ differs, so :extend seems redundant. Breakdown in footnote[1]. (Have been going back and forth over this; apologies if I've missed something and that conclusion is wrong) > Can you see if any of these changes cause any trouble in our own use > of face-differs-from-default-p? AFAICT, it will actually fix a subtle > problem in diff-mode.el: if diff-changed face doesn't define > non-default colors, it will be still taken as different from > 'default', which I think is contrary to what diff-mode expects. Will do; that was on my checklist[2] whichever solution we eventually pick. > If we make the above change in face-differs-from-default-p, would > using it for the purpose of this issue fix the problem? I think so. Scribbled the attached patch; will report once I've tested it more thoroughly against (a) the :inverse-video use-case from my OP, (b) the "region with unspecified :background" use-case from emacs-devel, (c) other in-tree users of face-differs-from-default-p. In the meantime, let me know if it looks good in principle; there are still details I'd like to tweak FWIW. E.g. * face-differs-from-default-p probably deserves a comment explaining what's going on wrt these attributes, * 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? >> (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) > But does this affect my proposal above? Not AFAICT. Good thing I hit message-send-and-exit in time =F0=9F=A4=95 Bottomline: let me know if the attached seems to go in the right direction; meanwhile, will check that it does TRT for other in-tree users of face-differs-from-default-p. Thanks for your patience. [1] Face under test has=E2=80=A6 | Does text with that face st= and out? :background =3D default :extend nil | no :background =3D default :extend t | no :background =E2=89=A0 default :extend nil | yes :background =E2=89=A0 default :extend t | yes =E2=87=92 face-differs-from-default-p can just check :background =E2=89=A0 = default, which it already does via display-supports-face-attributes-p. =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20 Face under test has=E2=80=A6 | Does text with that face st= and out? :underline =3D default :extend nil | no :underline =3D default :extend t | no :underline =E2=89=A0 default :extend nil | yes :underline =E2=89=A0 default :extend t | yes =E2=87=92 face-differs-from-default-p can just check :underline =E2=89=A0 d= efault, which it already does via display-supports-face-attributes-p. NB: as far as I tested, this is is true for :underline's boolean values as well as for its other forms. [2] Another thing on that checklist would be adding ERT testcases for face-differs-from-default-p, but I see the specter of --batch beaming with mischievous glee at the thought of me invoking display-related functions with a "headless" Emacs. Is it=E2=80=A6? - yep, yep, it *is* put= ting popcorn in the oven. =F0=9F=98=AE=E2=80=8D=F0=9F=92=A8 Maybe this time I can convince it to just eat the popcorn and not throw it at me. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-Avoid-spurious-pause-in-kill-ring-save.patch >From 033dec727e6eb381cae288aaacbce63eba76a7bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= Date: Sun, 15 Jan 2023 19:23:17 +0100 Subject: [PATCH] Avoid spurious pause in kill-ring-save 'indicate-copied-region' checks whether the region is "highlighted" and if not, briefly moves point to mark to give a visual cue of the extent of text that was saved to the kill ring. The region is considered "highlighted" if (a) it is active and (b) its face specifies a :background. That latter condition does not account for the multiple ways in which the face can make the region "visually distinct" from the default face, so switch to a more extensive predicate. * lisp/simple.el (region-highlighted-p): New function to detect "if the region is highlighted", leveraging display-supports-face-attributes-p. (indicate-copied-region): Use it. * lisp/faces.el (face-differs-from-default-p): Also ignore :extend, since the answers display-supports-face-attributes-p gives for that attribute do not help determine whether FACE is visually distinct from default. --- lisp/faces.el | 4 ++-- lisp/simple.el | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lisp/faces.el b/lisp/faces.el index 3323eab205..53a4ee5e09 100644 --- a/lisp/faces.el +++ b/lisp/faces.el @@ -303,8 +303,8 @@ face-differs-from-default-p If the optional argument FRAME is given, report on face FACE in that frame. If FRAME is t, report on the defaults for face FACE (for new frames). If FRAME is omitted or nil, use the selected frame." - (let ((attrs - (delq :inherit (mapcar 'car face-attribute-name-alist))) + (let ((attrs (seq-difference (mapcar 'car face-attribute-name-alist) + '(:extend :inherit))) (differs nil)) (while (and attrs (not differs)) (let* ((attr (pop attrs)) diff --git a/lisp/simple.el b/lisp/simple.el index 844cfa68b0..e2828caf39 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -5865,6 +5865,13 @@ copy-region-blink-delay :group 'killing :version "28.1") +(defun region-highlighted-p () + "Say whether the region is visibly highlighted. +This takes into account whether the region is active, and whether +the `region' face displays differently from the default face." + (and (region-active-p) + (face-differs-from-default-p 'region))) + (defun indicate-copied-region (&optional message-len) "Indicate that the region text has been copied interactively. If the mark is visible in the selected window, blink the cursor between @@ -5885,8 +5892,7 @@ indicate-copied-region ;; was selected. Don't do it if the region is highlighted. (when (and (numberp copy-region-blink-delay) (> copy-region-blink-delay 0) - (or (not (region-active-p)) - (not (face-background 'region nil t)))) + (not (region-highlighted-p))) ;; Swap point and mark. (set-marker (mark-marker) (point) (current-buffer)) (goto-char mark) -- 2.39.0 --=-=-=--