GNU bug report logs - #64347
30.0.50; Some customize faces shown as edited with -Q

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 64347 in the body.
You can then email your comments to 64347 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Some customize faces shown as edited with -Q
Date: Thu, 29 Jun 2023 12:15:00 +0200

0. emacs -Q
1. M-x customize-face RET RET
2. Toggle all face entries in the buffer *Customize Faces* (e.g. by
   creating this keyboard macro: C-s C-q C-j S RET C-f and then
   executing it 164 times) and search for the string "EDITED" in the
   buffer.
   => The following faces show the State "EDITED, shown value does not
   take effect until you set or save it.":
   confusingly-reordered
   custom-button
   custom-button-mouse
   custom-button-pressed
   mode-line
   mode-line-highlight
   mode-line-inactive
   tab-bar-tab
   tool-bar
   All other faces show the State "STANDARD".
3. Clicking the State button of these faces and selecting either "Undo
   Edits" or "Revert This Session's Customization" does not change the
   State shown.
4. Clicking the State button of, e.g., mode-line and selecting "Set for
   Current Session" changes the State shown to "SET for current session
   only."  I see no difference in the appearance of the mode line before
   and after this State change.
5. Clicking the State button of mode-line again and selecting "Revert
   This Session's Customization" changes the State shown back to
   "EDITED, shown value does not take effect until you set or save it.",
   and again the appearance of the mode-line is unchanged.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.17.6) of 2023-06-27 built on strobelfssd
Repository revision: cf4ccc58284de50959ea66b1cd2655ab2fa4d15b
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: Linux From Scratch r11.3-100-systemd

Configured using:
 'configure -C --with-xwidgets 'CFLAGS=-Og -g3'
 PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM
XINPUT2 XPM XWIDGETS GTK3 ZLIB

Message #8 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stephen Berman <stephen.berman <at> gmx.net>,
 Mauro Aranda <maurooaranda <at> gmail.com>
Cc: 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Thu, 29 Jun 2023 15:38:29 +0300

> From: Stephen Berman <stephen.berman <at> gmx.net>
> Date: Thu, 29 Jun 2023 12:15:00 +0200
> 
> 0. emacs -Q
> 1. M-x customize-face RET RET
> 2. Toggle all face entries in the buffer *Customize Faces* (e.g. by
>    creating this keyboard macro: C-s C-q C-j S RET C-f and then
>    executing it 164 times) and search for the string "EDITED" in the
>    buffer.
>    => The following faces show the State "EDITED, shown value does not
>    take effect until you set or save it.":
>    confusingly-reordered
>    custom-button
>    custom-button-mouse
>    custom-button-pressed
>    mode-line
>    mode-line-highlight
>    mode-line-inactive
>    tab-bar-tab
>    tool-bar
>    All other faces show the State "STANDARD".
> 3. Clicking the State button of these faces and selecting either "Undo
>    Edits" or "Revert This Session's Customization" does not change the
>    State shown.
> 4. Clicking the State button of, e.g., mode-line and selecting "Set for
>    Current Session" changes the State shown to "SET for current session
>    only."  I see no difference in the appearance of the mode line before
>    and after this State change.
> 5. Clicking the State button of mode-line again and selecting "Revert
>    This Session's Customization" changes the State shown back to
>    "EDITED, shown value does not take effect until you set or save it.",
>    and again the appearance of the mode-line is unchanged.

This is a regression between Emacs 27.2 and Emacs 28.1.  Bisecting
will be welcome.

Message #11 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stephen Berman <stephen.berman <at> gmx.net>, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Fri, 30 Jun 2023 08:33:39 -0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stephen Berman <stephen.berman <at> gmx.net>
>> Date: Thu, 29 Jun 2023 12:15:00 +0200
>>
>> 0. emacs -Q
>> 1. M-x customize-face RET RET
>> 2. Toggle all face entries in the buffer *Customize Faces* (e.g. by
>>    creating this keyboard macro: C-s C-q C-j S RET C-f and then
>>    executing it 164 times) and search for the string "EDITED" in the
>>    buffer.
>>    => The following faces show the State "EDITED, shown value does not
>>    take effect until you set or save it.":
>>    confusingly-reordered
>>    custom-button
>>    custom-button-mouse
>>    custom-button-pressed
>>    mode-line
>>    mode-line-highlight
>>    mode-line-inactive
>>    tab-bar-tab
>>    tool-bar
>>    All other faces show the State "STANDARD".
>> 3. Clicking the State button of these faces and selecting either "Undo
>>    Edits" or "Revert This Session's Customization" does not change the
>>    State shown.
>> 4. Clicking the State button of, e.g., mode-line and selecting "Set for
>>    Current Session" changes the State shown to "SET for current session
>>    only."  I see no difference in the appearance of the mode line before
>>    and after this State change.
>> 5. Clicking the State button of mode-line again and selecting "Revert
>>    This Session's Customization" changes the State shown back to
>>    "EDITED, shown value does not take effect until you set or save it.",
>>    and again the appearance of the mode-line is unchanged.
>
> This is a regression between Emacs 27.2 and Emacs 28.1. Bisecting
> will be welcome.

I tried to bisect but I'm finding build errors on older commits:
CC       sysdep.o
sysdep.c:1784:22: error: variably modified ‘sigsegv_stack’ at file scope
 1784 | static unsigned char sigsegv_stack[SIGSTKSZ];


So I did some debugging.  I noted that all the faces posted by Stephen
(except confusingly-reordered) have a Horizontal Width widget.  So
something like this is enough to get Custom confused:
(defface test
  '((t :box (:line-width 2 :style released-button)))
  "...")

M-x customize-face RET test
Shows the EDITED State.

So that points to Custom fiddling with the real value, i.e., what
face-attribute would return, but not with the "customized value", the
value that holds the Widget.

Looking at the changes in custom-face-attributes, I see this commit:
commit 34ae2d0c220c945443e94a43d043a4a63c444bf4
Author: Alexandre Adolphe <alexandre.adolphe <at> gmail.com>
Date:   Sat Aug 10 22:57:24 2019 +0200

    Allow negative line width for :box face attribute

And I noticed that it modified the real-value filter, but not the
customized-value filter.  So I suspect that might be the problem.

Maybe someone that is able to build Emacs for that and previous commits
can confirm.

In the meantime, I'll read the documentation on :line-width, since I'm
pretty sure a changed in the customized value filter is required.

(And I don't know what's wrong with the confusingly-reordered face yet)

Message #14 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Mauro Aranda <maurooaranda <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: Stephen Berman <stephen.berman <at> gmx.net>,
 "64347 <at> debbugs.gnu.org" <64347 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#64347: 30.0.50; Some customize faces shown as
 edited with -Q
Date: Fri, 30 Jun 2023 12:43:51 +0000

Thanks for saying what you did to try to
track this down (steps, logic).  Even for
people such as me, who don't help on this,
this helps us understand better.

Message #17 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: 64347 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stephen Berman <stephen.berman <at> gmx.net>
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Fri, 30 Jun 2023 11:05:33 -0300

Mauro Aranda <maurooaranda <at> gmail.com> writes:

> (And I don't know what's wrong with the confusingly-reordered face yet)

OK, some information about the confusingly-reordered face.  Take the
definition of the face, only change the name.
(defface test
  '((((supports :underline (:style wave)))
     :underline (:style wave :color "Red1"))
    (t
     :inherit warning))
  "...")

And evaluate that in Emacs 27, with -Q.  Then:
M-x customize-face RET test
Notice that it says Edited, and that the Widget shows the option to
modify Underline, Color, Style, as expected.

Now take the same definition to Emacs 29, with -Q and do the same.
The State is still EDITED (which is wrong, and the cause of the bug
report), but now you don't see the option to modify the Underline.
Somehow it didn't match.  So there's something different (and still
wrong) here going on.  I took a look at why the matching has changed and
it looks to me like this code in custom-face-attributes is responsible:
     ,(lambda (real-value)
    (and real-value
         (let ((color
            (or (and (consp real-value) (plist-get real-value :color))
                (and (stringp real-value) real-value)
                'foreground-color))
           (style
            (or (and (consp real-value) (plist-get real-value :style))
                'line))
                   (position (and (consp real-value)
                                  (plist-get real-value :style))))
               (nconc (and color  `(:color ,color))
                      `(:style ,style)
                      (and position `(:position ,position))))))

(plist-get real-value :style) for position looks like a typo introduced
in:
commit 4f50d964e51bbe5219f40df4353f4314c7ade985
Author: Po Lu <luangruo <at> yahoo.com>
Date:   Mon Jan 10 19:26:46 2022 +0800

    Allow controlling the underline position of faces

Changing that line to (plist-get real-value :position) brings back the
correct match, so that's one less bug.

Still, the important one is left.  Now, go back in time to Emacs 27
again, but slightly change the definition for test:
(defface test
  '((((supports :underline (:style wave)))
     :underline (:color "Red1" :style wave))
    (t
     :inherit warning))
  "...")

Notice the properties in :underline were swapped.
M-x customize-face RET test
Shows the Standard State, as expected.  So it seems to me that something
somewhere is checking for list equality where it should check for plist
equality.


With the previous typo fix, go back to Emacs 29 and repeat.  It still
shows EDITED as the State, so again, there's something else wrong here.
I think what was left out in terms of the :underline property is that
:position doesn't have to be always specified.  Changing the filter for
the customized-value to return something like this:
(nconc `(:color ,color) `(:style ,style) (and position `(:position 
,position)))
fixes it, now customizing the unedited face shows STANDARD as the state.


To sum it up, I think there are bugs in custom-face-attributes. One is
most surely a typo, and the other ones are oversights in the filters for
the :underline and :box properties.  Fixing those, we are left with one
bug, I think, that will be reproducible with Emacs -Q and evaluating:

(defface test
  '((((supports :underline (:style wave)))
     :underline (:color "Red1" :style wave))
    (t
     :inherit warning))
  "...")

(defface test-2
  '((((supports :underline (:style wave)))
     :underline (:style wave :color "Red1"))
    (t
     :inherit warning))
  "...")

M-x customize-face RET test
will show STANDARD state

while
M-x customize-face RET test-2
will show EDITED state

Message #20 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Sat, 08 Jul 2023 10:49:53 +0300

> Date: Fri, 30 Jun 2023 11:05:33 -0300
> From: Mauro Aranda <maurooaranda <at> gmail.com>
> Cc: Stephen Berman <stephen.berman <at> gmx.net>, Eli Zaretskii <eliz <at> gnu.org>
> 
> To sum it up, I think there are bugs in custom-face-attributes. One is
> most surely a typo, and the other ones are oversights in the filters for
> the :underline and :box properties.  Fixing those, we are left with one
> bug, I think, that will be reproducible with Emacs -Q and evaluating:
> 
> (defface test
>    '((((supports :underline (:style wave)))
>       :underline (:color "Red1" :style wave))
>      (t
>       :inherit warning))
>    "...")
> 
> (defface test-2
>    '((((supports :underline (:style wave)))
>       :underline (:style wave :color "Red1"))
>      (t
>       :inherit warning))
>    "...")
> 
> M-x customize-face RET test
> will show STANDARD state
> 
> while
> M-x customize-face RET test-2
> will show EDITED state

Thanks.

Can you show a patch for the two bugs you've succeeded to identify?

Did you make any progress with the one bug that's left after the other
two are fixed?

Message #23 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Sat, 8 Jul 2023 18:32:15 -0300

Eli Zaretskii <eliz <at> gnu.org> writes:

> Thanks.
>
> Can you show a patch for the two bugs you've succeeded to identify?

Not yet.  I wrote a working post-filter for the :box property, but
there are still some faces that show with the EDITED state.

Since there are both ways to specify the same information for the
:line-width value, there's always a chance of not sending the expected
format to face-spec-match-p.

Let's say we have (:line-width 1).  Then the pre-filter converts it into
(:line-width (1 . 1)), and then the post-filter should convert it back
to (:line-width 1), and that works.

But if we have (:line-width (1 . 1)), the pre-filter does nothing, and
the post-filter will transform it into (:line-width 1), and that way
specs won't match.

So sometimes we want to post-filter to (:line-width WIDTH) and sometimes
to (VWIDTH . HWIDTH) and there's currently not enough information to do
that, or at least I missed it.

> Did you make any progress with the one bug that's left after the other
> two are fixed?

I did find that face-attr-match-p uses equal even with properties like
:underline and :box that can have plists as values.  But I haven't tried
to fix it, because I got frustrated with the post-filter thing.

Message #26 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Sun, 09 Jul 2023 08:43:58 +0300

> Date: Sat, 8 Jul 2023 18:32:15 -0300
> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
> From: Mauro Aranda <maurooaranda <at> gmail.com>
> 
> Since there are both ways to specify the same information for the
> :line-width value, there's always a chance of not sending the expected
> format to face-spec-match-p.
> 
> Let's say we have (:line-width 1).  Then the pre-filter converts it into
> (:line-width (1 . 1)), and then the post-filter should convert it back
> to (:line-width 1), and that works.
> 
> But if we have (:line-width (1 . 1)), the pre-filter does nothing, and
> the post-filter will transform it into (:line-width 1), and that way
> specs won't match.
> 
> So sometimes we want to post-filter to (:line-width WIDTH) and sometimes
> to (VWIDTH . HWIDTH) and there's currently not enough information to do
> that, or at least I missed it.
> 
>  > Did you make any progress with the one bug that's left after the other
>  > two are fixed?
> 
> I did find that face-attr-match-p uses equal even with properties like
> :underline and :box that can have plists as values.  But I haven't tried
> to fix it, because I got frustrated with the post-filter thing.

I guess I'm missing something here: why do we need those pre-filter
and post-filter conversions?  The C code understands both forms of
:line-width, so there should be no need for Lisp to do any
conversions, right?  So why do we do that? why not simply leave the
spec as it was originally?

Message #29 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Sun, 9 Jul 2023 08:44:23 -0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sat, 8 Jul 2023 18:32:15 -0300
>> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>>
>> Since there are both ways to specify the same information for the
>> :line-width value, there's always a chance of not sending the expected
>> format to face-spec-match-p.
>>
>> Let's say we have (:line-width 1).  Then the pre-filter converts it into
>> (:line-width (1 . 1)), and then the post-filter should convert it back
>> to (:line-width 1), and that works.
>>
>> But if we have (:line-width (1 . 1)), the pre-filter does nothing, and
>> the post-filter will transform it into (:line-width 1), and that way
>> specs won't match.
>>
>> So sometimes we want to post-filter to (:line-width WIDTH) and sometimes
>> to (VWIDTH . HWIDTH) and there's currently not enough information to do
>> that, or at least I missed it.
>>
>>  > Did you make any progress with the one bug that's left after the 
other
>>  > two are fixed?
>>
>> I did find that face-attr-match-p uses equal even with properties like
>> :underline and :box that can have plists as values.  But I haven't tried
>> to fix it, because I got frustrated with the post-filter thing.
>
> I guess I'm missing something here: why do we need those pre-filter
> and post-filter conversions?  The C code understands both forms of
> :line-width, so there should be no need for Lisp to do any
> conversions, right?  So why do we do that? why not simply leave the
> spec as it was originally?

Custom needs the pre-filter in order to present a Custom buffer to edit
the face.
Let's say there's a face:
(defface foo
  '((t (:box (:line-width 1 :color "black"))))
  "...")

And let's say a user wants to customize it via Custom.
M-x customize-face RET foo
should show the user a buffer with all the capabilities to edit it.
Because we have an integer for the :line-width property, the user will
be presented with an integer Widget to edit the value, without giving
the user the opportunity to edit the HWIDTH and VWIDTH separately.

So the pre-filter takes the (:line-width 1), and converts it into
(:line-width (1 . 1)), and now the Custom buffer will have
a cons Widget.  If we didn't do that conversion, that would be a Bug
report, I'm sure.

Of course, if the face is defined like so:
(defface foo
  '((t (:box (:line-width (1 . 1) :color "black"))))
  "...")
then the pre-filter doesn't have to do anything.  So, sometimes it does
something, sometimes not, but I hope I've convinced you about why is
needed.

Now, your question about the post-filter made me question its need a
little bit.  At first sight, I thought it was clear than a post-filter
was needed too.  As I said in my first message, Custom is fiddling with
the value for :line-width, but should present it to the face-spec-match-p
"unfiltered".  And currently, that's a need because when it comes to the
face-attr-match-p function, it will use equal to try to see if
attributes in two specs match.  So the post-filter should take
(:line-width (1 . 1)) and turn it back into (:line-width 1)

That's where I was originally, before your mail.  Now I think that maybe
face-attr-match-p could do something slightly smarter when comparing
these values, since, as you say, the C code understands both forms.
IOW, maybe the fix is that face-attr-match-p shouldn't produce a
mismatch between specs that have these both forms being compared.

Message #32 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Sun, 09 Jul 2023 15:13:10 +0300

> Date: Sun, 9 Jul 2023 08:44:23 -0300
> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
> From: Mauro Aranda <maurooaranda <at> gmail.com>
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>  > I guess I'm missing something here: why do we need those pre-filter
>  > and post-filter conversions?  The C code understands both forms of
>  > :line-width, so there should be no need for Lisp to do any
>  > conversions, right?  So why do we do that? why not simply leave the
>  > spec as it was originally?
> 
> Custom needs the pre-filter in order to present a Custom buffer to edit
> the face.
> Let's say there's a face:
> (defface foo
>    '((t (:box (:line-width 1 :color "black"))))
>    "...")
> 
> And let's say a user wants to customize it via Custom.
> M-x customize-face RET foo
> should show the user a buffer with all the capabilities to edit it.
> Because we have an integer for the :line-width property, the user will
> be presented with an integer Widget to edit the value, without giving
> the user the opportunity to edit the HWIDTH and VWIDTH separately.
> 
> So the pre-filter takes the (:line-width 1), and converts it into
> (:line-width (1 . 1)), and now the Custom buffer will have
> a cons Widget.  If we didn't do that conversion, that would be a Bug
> report, I'm sure.

OK, but why does it have to do that on the original value?  It could
do that on a copy that serves for the display and editing, in which
case the original value could be left intact if the user didn't change
it or did change, but didn't click Apply.  (If the user does modify
the original value, then any conversions are okay, since the variable
is really "edited".)

Message #35 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Sun, 9 Jul 2023 20:12:38 -0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sun, 9 Jul 2023 08:44:23 -0300
>> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>  > I guess I'm missing something here: why do we need those pre-filter
>>  > and post-filter conversions?  The C code understands both forms of
>>  > :line-width, so there should be no need for Lisp to do any
>>  > conversions, right?  So why do we do that? why not simply leave the
>>  > spec as it was originally?
>>
>> Custom needs the pre-filter in order to present a Custom buffer to edit
>> the face.
>> Let's say there's a face:
>> (defface foo
>>    '((t (:box (:line-width 1 :color "black"))))
>>    "...")
>>
>> And let's say a user wants to customize it via Custom.
>> M-x customize-face RET foo
>> should show the user a buffer with all the capabilities to edit it.
>> Because we have an integer for the :line-width property, the user will
>> be presented with an integer Widget to edit the value, without giving
>> the user the opportunity to edit the HWIDTH and VWIDTH separately.
>>
>> So the pre-filter takes the (:line-width 1), and converts it into
>> (:line-width (1 . 1)), and now the Custom buffer will have
>> a cons Widget.  If we didn't do that conversion, that would be a Bug
>> report, I'm sure.
>
> OK, but why does it have to do that on the original value? It could
> do that on a copy that serves for the display and editing, in which
> case the original value could be left intact if the user didn't change
> it or did change, but didn't click Apply.  (If the user does modify
> the original value, then any conversions are okay, since the variable
> is really "edited".)

I think my description was inaccurate, because it seemed to imply that
it is a destructive operation.  It is not, it leaves the original value
intact.

But when deciding to set a state, Custom always consults the spec built
from the data the face Widget has.

Message #38 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Mon, 10 Jul 2023 15:47:53 +0300

> Date: Sun, 9 Jul 2023 20:12:38 -0300
> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
> From: Mauro Aranda <maurooaranda <at> gmail.com>
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>  > OK, but why does it have to do that on the original value? It could
>  > do that on a copy that serves for the display and editing, in which
>  > case the original value could be left intact if the user didn't change
>  > it or did change, but didn't click Apply.  (If the user does modify
>  > the original value, then any conversions are okay, since the variable
>  > is really "edited".)
> 
> I think my description was inaccurate, because it seemed to imply that
> it is a destructive operation.  It is not, it leaves the original value
> intact.
> 
> But when deciding to set a state, Custom always consults the spec built
> from the data the face Widget has.

Can we change this last aspect, so that the state is set using the
original spec if the setting was not changed by the user?

Message #41 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Mon, 10 Jul 2023 10:45:38 -0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sun, 9 Jul 2023 20:12:38 -0300
>> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>  > OK, but why does it have to do that on the original value? It could
>>  > do that on a copy that serves for the display and editing, in which
>>  > case the original value could be left intact if the user didn't 
change
>>  > it or did change, but didn't click Apply.  (If the user does modify
>>  > the original value, then any conversions are okay, since the variable
>>  > is really "edited".)
>>
>> I think my description was inaccurate, because it seemed to imply that
>> it is a destructive operation.  It is not, it leaves the original value
>> intact.
>>
>> But when deciding to set a state, Custom always consults the spec built
>> from the data the face Widget has.
>
> Can we change this last aspect, so that the state is set using the
> original spec if the setting was not changed by the user?

That might be possible.  I will check if it doesn't lead to any troubles
and if it actually fixes these inconsistencies.

Message #44 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Sat, 15 Jul 2023 17:01:44 -0300

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sun, 9 Jul 2023 20:12:38 -0300
>> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>  > OK, but why does it have to do that on the original value? It could
>>  > do that on a copy that serves for the display and editing, in which
>>  > case the original value could be left intact if the user didn't 
change
>>  > it or did change, but didn't click Apply.  (If the user does modify
>>  > the original value, then any conversions are okay, since the variable
>>  > is really "edited".)
>>
>> I think my description was inaccurate, because it seemed to imply that
>> it is a destructive operation.  It is not, it leaves the original value
>> intact.
>>
>> But when deciding to set a state, Custom always consults the spec built
>> from the data the face Widget has.
>
> Can we change this last aspect, so that the state is set using the
> original spec if the setting was not changed by the user?

OK, here's a patch for doing that.  It seems to me that after creating
the widget, the only reason to use the value that's represented in the
widget is if we loaded the spec from the :shown-value property (meaning
we are redrawing the widget and we want to keep a spec that was already
in the widget)

[0001-Pass-original-spec-just-after-creating-the-face-widg.patch (text/x-patch, attachment)]

Message #47 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Sat, 15 Jul 2023 17:11:21 -0300

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Fri, 30 Jun 2023 11:05:33 -0300
>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>> Cc: Stephen Berman <stephen.berman <at> gmx.net>, Eli Zaretskii 
<eliz <at> gnu.org>
>>
>> To sum it up, I think there are bugs in custom-face-attributes. One is
>> most surely a typo, and the other ones are oversights in the filters for
>> the :underline and :box properties.  Fixing those, we are left with one
>> bug, I think, that will be reproducible with Emacs -Q and evaluating:
>>
>> (defface test
>>    '((((supports :underline (:style wave)))
>>       :underline (:color "Red1" :style wave))
>>      (t
>>       :inherit warning))
>>    "...")
>>
>> (defface test-2
>>    '((((supports :underline (:style wave)))
>>       :underline (:style wave :color "Red1"))
>>      (t
>>       :inherit warning))
>>    "...")
>>
>> M-x customize-face RET test
>> will show STANDARD state
>>
>> while
>> M-x customize-face RET test-2
>> will show EDITED state
>
> Thanks.
>
> Can you show a patch for the two bugs you've succeeded to identify?
>
> Did you make any progress with the one bug that's left after the other
> two are fixed?

Here's a patch for the typo.

Concerning the other bugs I discovered, I think that while the filters
could be tweaked, a better fix would be to teach face-spec-match-p
about matching plists correctly and not just by equality.

[0001-Fix-typo-in-pre-filter-for-underline-property.patch (text/x-patch, attachment)]

Message #50 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Thu, 20 Jul 2023 18:45:46 +0300

> Date: Sat, 15 Jul 2023 17:01:44 -0300
> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
> From: Mauro Aranda <maurooaranda <at> gmail.com>
> 
>  > Can we change this last aspect, so that the state is set using the
>  > original spec if the setting was not changed by the user?
> 
> OK, here's a patch for doing that.  It seems to me that after creating
> the widget, the only reason to use the value that's represented in the
> widget is if we loaded the spec from the :shown-value property (meaning
> we are redrawing the widget and we want to keep a spec that was already
> in the widget)

Thanks, installed on master.

Message #53 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Thu, 20 Jul 2023 18:49:44 +0300

> Date: Sat, 15 Jul 2023 17:11:21 -0300
> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
> From: Mauro Aranda <maurooaranda <at> gmail.com>
> 
> Here's a patch for the typo.

Thanks, installed on the emacs-29 branch.

> Concerning the other bugs I discovered, I think that while the filters
> could be tweaked, a better fix would be to teach face-spec-match-p
> about matching plists correctly and not just by equality.

Probably.

Do we have anything left to do in this bug, or could it be closed?

Message #56 received at 64347 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stephen.berman <at> gmx.net, 64347 <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Thu, 20 Jul 2023 15:56:02 -0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Concerning the other bugs I discovered, I think that while the filters
>> could be tweaked, a better fix would be to teach face-spec-match-p
>> about matching plists correctly and not just by equality.
>
> Probably.
>
> Do we have anything left to do in this bug, or could it be closed?

I think this can be closed.

Message #61 received at 64347-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: stephen.berman <at> gmx.net, 64347-done <at> debbugs.gnu.org
Subject: Re: bug#64347: 30.0.50; Some customize faces shown as edited with -Q
Date: Thu, 20 Jul 2023 22:09:09 +0300

> Date: Thu, 20 Jul 2023 15:56:02 -0300
> Cc: 64347 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
> From: Mauro Aranda <maurooaranda <at> gmail.com>
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>  > Do we have anything left to do in this bug, or could it be closed?
> 
> I think this can be closed.

Thanks, done.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.