From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stephen Berman via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#69941: 30.0.50; Faulty fontification of radio button widgets Date: Mon, 01 Apr 2024 17:20:40 +0200 Message-ID: <8734s5w1mf.fsf@gmx.net> References: <87h6gynx49.fsf@rub.de> <865xxe1dwd.fsf@gnu.org> <51c20b56-4b82-4f5c-9559-cdbd0146df22@gmail.com> <87wmprqxj3.fsf@gmx.net> Reply-To: Stephen Berman 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="23460"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Eli Zaretskii , 69941@debbugs.gnu.org, Stefan Monnier To: Mauro Aranda Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Apr 01 17:21:27 2024 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 1rrJTT-0005yA-6A for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 01 Apr 2024 17:21:27 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rrJT3-0008MV-Px; Mon, 01 Apr 2024 11:21:02 -0400 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 1rrJT1-0008Ll-Fa for bug-gnu-emacs@gnu.org; Mon, 01 Apr 2024 11:20:59 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rrJT1-0001rW-7Q for bug-gnu-emacs@gnu.org; Mon, 01 Apr 2024 11:20:59 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rrJT4-0003g2-2q for bug-gnu-emacs@gnu.org; Mon, 01 Apr 2024 11:21:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stephen Berman Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 01 Apr 2024 15:21:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 69941 X-GNU-PR-Package: emacs Original-Received: via spool by 69941-submit@debbugs.gnu.org id=B69941.171198485814117 (code B ref 69941); Mon, 01 Apr 2024 15:21:02 +0000 Original-Received: (at 69941) by debbugs.gnu.org; 1 Apr 2024 15:20:58 +0000 Original-Received: from localhost ([127.0.0.1]:51374 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rrJSz-0003fc-7t for submit@debbugs.gnu.org; Mon, 01 Apr 2024 11:20:57 -0400 Original-Received: from mout.gmx.net ([212.227.15.19]:51073) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rrJSx-0003fP-M8 for 69941@debbugs.gnu.org; Mon, 01 Apr 2024 11:20:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmx.net; s=s31663417; t=1711984842; x=1712589642; i=stephen.berman@gmx.net; bh=ff80F5P1vKxQG38joAlolyhVVnhfX3PY0zV595rYNwE=; h=X-UI-Sender-Class:From:To:Cc:Subject:In-Reply-To:References: Date; b=raQo+S2tQAJcm1WzTip3r1qVY79R0BtFcGLqRC/0mM2GoIruRhgmvfMjZNrQOZM7 K1jpaYe8Cnroq91DzG3x1rnYUIvMx8qgHE8Srdm11JRWwk2/xNJbwi2BuBashxQny 2gbrcuztCoK4jMd36Y90OyUuRYfWPW1MoF7Ef6M4XbKOUzYr3qNim3Oruz0BNc5qq 8wJm5G2tC0jNNrUgkqsk8RWg+xzlzV5aFtsUDSfcyDFZcs740AGB5onxSj3iUIWAA V5HbpPJgFT4Ozhhh/EvgMyrYxFfxckQa9jrq1sJPpwmEM+PB49l5cx+5KubcEVOYP 9S2JbZlAjMOaNvsTVw== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Original-Received: from strobelfs ([94.134.95.171]) by mail.gmx.net (mrgmx004 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MNbp3-1sAlKD0bql-00P8Ad; Mon, 01 Apr 2024 17:20:42 +0200 In-Reply-To: <87wmprqxj3.fsf@gmx.net> (Stephen Berman's message of "Sun, 24 Mar 2024 19:45:20 +0100") X-Provags-ID: V03:K1:+2qLvW7p7pyS2JmciX6bIMpUPvIkmwZFTfdCvWPE2uomr6jtnk9 XqgiyDbbRTH7bHZ7Icy1fAWKOweNGKx1Zyee5cqIRAUlRyyzd9mk6CzJw0b2K3caweE2VUh FzCQtxYPMz7A0ov3kgr0E9s+OIvzevpooZ7lxNLX7A5eip9BuYUvled5bM1IEnUToiamBjm MBfidk/RdVvhSz2AySG8A== UI-OutboundReport: notjunk:1;M01:P0:Vnr9gZ1dvVc=;aHEYg5SoFGlRJ0f4M5BDqHfULyf 66YtKzSSUpF9rfeKvlpBMTz+0PXx4Yv1+jaEBY0JNrrxQ/h69MB3zzVfA+aSGuz0GVtvbmXv3 66EpFd+MgmXEd7sNDBOS1YNceCV++YI4ztSaGiU9ejetqdc6wmQ5fhQkuUrI/BKd5umL/PqwB MacrtLa3xRoI/wbJu1KBRwuG0rdRPJ4kdqH8TcgnVN2RmQO55j2E/Ln5g34s1QpvfqZxxLdeG f7eDJDfULGwLMbLNwjpvV5+CcqWVmtUIviCsoogWg+CmJF/I0Ba0EteImB5XTWCXWpnGJj/nk aTLbTrI3hfO1OQG8lk0ZlPdMIBiYvIBnqUjVge1FIRd5kwx8umZ1SIpRRZ6y1e20GdABwun4Z p1fQt4kfMp0v41MxZcHKvexjyekQGJoyHWIAyMioDYhd4mfTZ/xpg0AI/q7n+A25dRS1HBqXd b41pGOoRHjvJz9ULCrSwccn90A9Lg5JqnJ1YpGXpoCfM9V0s9KPCSIArPbpx4DZXpAxwjI9NL 3qSdAdPJwEH3HyXPyi5UJvDPCq4tfGRjqEMxD5GbdDFfKD+XTCLqfG2rsMcprHvTaHYQrfn0B 7sfBEYng/Zk/MCVz++f7+AKKs/Eon9NnB/3UoXvNdWRFmiRjgUccTXqLCFrAumhPtM2e6fwbG 4Rjv7fXOaWw3w9AOqzhZdAbdXuXXInLp0Fl6LbSuh+BW4jB+xXelXl6hk7TEzmhdfglOnF35M CzcKTeseoCP3ve6K6/kRI3SJfyLij5xKYco7Lz41BYeKIgwSYMKZlbKXaJEtiP7OU/DeL8v5 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:282478 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Sun, 24 Mar 2024 19:45:20 +0100 Stephen Berman = wrote: > On Sat, 23 Mar 2024 17:49:53 -0300 Mauro Aranda = wrote: > >> Stephen Berman writes: >> >>> 5. Tab back to "Activate" and press RET, again restoring the initial >>> state.=C2=A0 Now tab to radio button "Two" and press RET. >>> =3D> The fontification is the same as in step 4: radio button "Two" has >>> the widget-inactive face but radio button "One" has the default (active) >>> face, though it is again inactive.=C2=A0 Repeatedly pressing either of = the >>> radio buttons (after activating them), does not change the fontification >>> of "One" again. >>> >>> >>> The faulty fontification of radio button "One" also obtains if there is >>> just one radio button instead of two, and if there are more than two >>> radio buttons, it is only the first one that displays the odd >>> fontification (admittedly, I've only test up to three radio buttons). >>> >>> I've tried to debug this and found that the problem seems to be due to >>> the sexp (set-marker-insertion-type from t) near the end of >>> widget-default-create, which advances the marker specified by the >>> widget's :from property.=C2=A0 Changing t to nil fixes the faulty >>> fontification of the first radio button. >>> >>> I investigated the history of this code, and while the value t for the >>> marker insertion type was used in the initial commit, it was changed to >>> nil in commit e0f956935, with the message "Insert new text at the :from >>> marker _after_ the marker, not before it."=C2=A0 But 18 days later it w= as >>> changed back to t in commit 3bff434b8, that also added "Document need to >>> put some text before the %v escape in :format string" of editable-field >>> widgets.=C2=A0 (I looked at the bug-gnu-emacs and emacs-devel mailing l= ist >>> archives but found nothing relevant at the time just prior to these >>> commits.) >> >> I'm pretty sure it makes sense for user-editable widgets that the >> value for insertion-type be t. > > Yes, if my understanding is correct, it's just radio-button-choice > widgets that need (the effect of) insertion type nil (at least for > setting the widget-inactive face), see below. > >>> So evidently the advancing marker insertion type is needed for at least >>> some widgets, though it seems to be problematic for radio buttons.=C2= =A0 So I >>> tried to conditionalize the choice of t or nil on the type of the >>> widget.=C2=A0 I used (not (eq 'radio-button (widget-type widget))), sin= ce the >>> argument `widget' of widget-default-create is, according to Edebug, >>> indeed radio-button, so negating the eq sexp returns nil, which I had >>> found to be the value of the marker insertion type that fixes the >>> fontification (however, I couldn't think of a way of limiting the >>> conditioning to only the first radio button, but in my testing so far >>> that lack doesn't appear to make a difference). >> >> I'm not sure if the right target is the radio-button widget.=C2=A0 It co= uld >> be the radio-button-choice widget.=C2=A0 Did you try to conditionalize t= he code >> against the radio-button-choice widget? > > I didn't, because I got hung up on the radio-button widget, since in > Edebug that is what I saw and (mistakenly) took to be the current widget > when widget-inactive face is set. But the resulting marker insertion > type discrepancy is really proof that I was looking at the wrong widget > type (as I already realized in my comments cited below, but I didn't > think to simply try it with radio-button-choice until now, so thanks for > pointing me in the right direction!). And indeed, with > radio-button-choice, negating the eq test DTRT, i.e., using (not (eq > 'radio-button-choice (widget-type widget))) as the condtion results in > the correct fontification. Since this sexp gives the > radio-button-choice widget's :from property the marker insertion type > nil, there is no discrepancy between using that sexp and directly using > nil, so changing my patch to use that condition would be in improvement. > Alternatively, ... > >>> But in fact, using the negation of the value of the eq sexp results in >>> the same faulty fontification, while omitting the negation (as in the >>> attached patch), which yields the advancing insertion type t, gives the >>> correct fontification, just like using nil does.=C2=A0 This makes no se= nse to >>> me, yet it is reliably reproducible.=C2=A0 The only possible explanatio= n that >>> occurs to me is that the bug is triggered elsewhere in the Emacs code >>> and somehow using the sexp that evaluates to t as the marker insertion >>> type affects that code, while using t itself does not (or rather, has >>> the opposite effect); but how that could be and where the culpable code >>> is, I don't know (as a guess, perhaps in the C code that adds faces, but >>> I don't know how to debug that).=C2=A0 If anyone knows or has an idea w= hat's >>> going on here, please communicate it.=C2=A0 In the meantime I will cont= inue >>> to use the widget library with the patch to see whether it has unwanted >>> consequences. >> >> I don't know much about that code in Emacs.=C2=A0 If we find some hack t= hat >> works maybe we can use that until someone figures it out.=C2=A0 But agai= n, >> given your analysis, I'd like to find out if using the condition on the >> radio-button-choice widget works as expected.=C2=A0 And of course, the h= ack >> shouldn't be added to the widget-default-create, which should remain >> type agnostic. > > ... since the issue is fontification with the widget-inactive face, > perhaps a better location for the condition is widget-specify-inactive, > as in the attached patch. It's still a hack though, since > widget-specify-inactive is also type-agnostic by design. But if the > issue really is confined to radio-button-choice widget's, I guess any > solution will have to refer to that type. However, between adding the > condition to widget-specify-inactive or to widget-default-create, I'm > not sure which is less hacky: since the patch to widget-default-create > effectively undoes the result of setting the marker insertion type to t, > perhaps it is cleaner just to set it to nil for radio-button-choice > widgets in widget-default-create. Or maybe someone will come up with a > better fix... > > Steve Berman > > diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el > index 172da3db1e0..01319853edc 100644 > --- a/lisp/wid-edit.el > +++ b/lisp/wid-edit.el > @@ -532,6 +532,17 @@ widget-inactive >=20=20 > (defun widget-specify-inactive (widget from to) > "Make WIDGET inactive for user modifications." > + ;; When WIDGET is a radio-button-choice widget and its first child > + ;; radio-button widget is inserted, the marker FROM, which has > + ;; insertion type t, advances to the position after the radio button, > + ;; and since the overlay setting the widget-inactive face begins at > + ;; the position of FROM, this results in the first radio button > + ;; incorrectly not being fontified with the widget-inactive face. To > + ;; ensure it is correctly fontified, we move FROM backward by 3, > + ;; i.e. the length of the radio-button widget (from its string > + ;; representation "( )" or "(x)") (bug#69941). > + (when (eq (widget-type widget) 'radio-button-choice) > + (set-marker from (- from 3))) > (unless (widget-get widget :inactive) > (let ((overlay (make-overlay from to nil t nil))) > (overlay-put overlay 'face 'widget-inactive) To fix this bug, do you have a preference between this patch for widget-specify-inactive and the attached patch for widget-default-create? Or do you have a better fix? Steve Berman --=-=-= Content-Type: text/x-patch Content-Disposition: attachment Content-Description: widget-default-create patch Content-Transfer-Encoding: quoted-printable diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el index 172da3db1e0..7fc9ac59b0a 100644 =2D-- a/lisp/wid-edit.el +++ b/lisp/wid-edit.el @@ -1733,8 +1733,17 @@ widget-default-create (goto-char value-pos) (widget-apply widget :value-create))) (let ((from (point-min-marker)) - (to (point-max-marker))) - (set-marker-insertion-type from t) + (to (point-max-marker)) + ;; When WIDGET is a radio-button-choice widget and its first + ;; child radio-button widget is inserted, advancing the marker + ;; FROM would make the overlay setting the widget-inactive face + ;; begin right after the first radio button, which would hence + ;; incorrectly not be fontified with the widget-inactive face. + ;; To ensure it is correctly fontified, we set the marker + ;; insertion type of FROM to nil only when WIDGET is + ;; radio-button-choice, otherwise to t (bug#69941). + (from-mit (not (eq 'radio-button-choice (widget-type widget))))) + (set-marker-insertion-type from from-mit) (set-marker-insertion-type to nil) (widget-put widget :from from) (widget-put widget :to to))) diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el index 4b049478b29..d416eb99022 100644 =2D-- a/test/lisp/wid-edit-tests.el +++ b/test/lisp/wid-edit-tests.el @@ -336,7 +336,13 @@ widget-test-widget-move (widget-forward 2) (forward-char) (widget-backward 1) - (should (string=3D "Second" (widget-value (widget-at)))))) + (should (string=3D "Second" (widget-value (widget-at)))) + ;; Check that moving to a widget at beginning of buffer does not + ;; signal a beginning-of-buffer error (bug#69943). + (widget-backward 1) ; Should not signal beginning-of-buffer error. + (widget-forward 2) + (should (string=3D "Third" (widget-value (widget-at)))) + (widget-forward 1))) ; Should not signal beginning-of-buffer error. (ert-deftest widget-test-color-match () "Test that the :match function for the color widget works." --=-=-=--