bug#57499: Documentation bug in the docstring of set-face-attribute?

[Eli Zaretskii]

> Date: Wed, 31 Aug 2022 21:13:58 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 57499@debbugs.gnu.org
> 
> > I don't see the difference between my text and these two variants. Why 
> > repeat that the value `unspecified' is a symbol `unspecified'?  We never 
> > say such tautological things in doc strings.
> 
> I just want to make it as clear as possible that to get that special value 
> `unspecified' one should use the symbol 'unspecified.

We have gazillions of such situations everywhere in Emacs where symbol
values are documented, and we never say anything beyond the name of
the symbol with proper quoting.

> > Read the discussion in bug#54156 again!  That's what it was about.
> >
> > Or read the code in xfaces which deals with value of unspecified when 
> > FRAME = t -- it doesn't just store the symbol 'unspecified' in the 
> > face's attribute, it does something more sneaky.  And it interprets nil 
> > as unspecified in some cases, but not in others.
> >
> > People stumble on these subtleties all the time, and the advice to use 
> > an explicit separate call with FRAME = t in the current doc string was 
> > intended to prevent that.  And note that it did work in Joost's case: he 
> > maybe didn't fully understand _why_ he needs to do it, but he did 
> > understand _how_ to do what he wanted.  Now we want to take that out, 
> > because it hurts our excessive sense of rigor, and we will get the same 
> > confusion back...
> 
> Hmmm...  Joost's conclusion was that using frame = nil and 'unspecified 
> solved his problem, and that he would do that.

Let me try explaining the issue which that part of the doc string
tries to address, one last time.

The text mentions 'defface'.  Why does it do that?  Because a face's
'defface' is relevant when realizing the faces of newly-created
frames; it has no effect whatsoever on existing frames.

So the issue here is: how do you reset a face's attribute to
'unspecified' in a way that would override what its 'defface' spec
says, and set the attribute to 'unspecified' on future frames?  The
doc string says that if you want to do that (and it is not clear that
you do, because faces are frame-local), _then_ you should call
set-face-attribute with FRAME = t and the attribute's value set to
'unspecified', and that call does special wizardry to ensure 'defface'
is overridden when new frames are created.

That is why the doc string mentions 'defface', and that's why it talks
specifically about new frames.  I thought mentioning both makes the
extra set-face-attribute call with FRAME = t natural and easier to
remember and apply.

So here's one more attempt to clarify the doc string without losing
that information:

  Set attributes of FACE on FRAME from ARGS.

  This function overrides the face attributes specified by FACE's face spec. 
  It is mostly intended for internal use.

  If FRAME is a frame, set the FACE's attributes only for that frame.  If
  FRAME is nil, set attribute values for all existing frames, as well as
  the default for new frames.  If FRAME is t, change the default values
  of attributes for new frames.

  ARGS must come in pairs ATTRIBUTE VALUE.  ATTRIBUTE must be a valid face 
  attribute name and VALUE must be a value that is valid for ATTRIBUTE,
  as described below for each attribute.

  In addition to the attribute values listed below, all attributes can
  also be set to the special value `unspecified', which means the face
  doesn't by itself specify a value for the attribute.

  When a new frame is created, attribute values in the FACE's
  `defspec' normally override the `unspecified' values in the FACE's
  default attributes.  To avoid that, i.e. to cause ATTRIBUTE's value
  be reset to `unspecified' when creating new frames, disregarding
  what the FACE's face spec says, call this function with FRAME set to
  t and the ATTRIBUTE's value set to `unspecified'.

> Just to be clear: I certainly do not want to take anything out.  I simply 
> do not understand (neither by testing nor by reading the code) what
> 
> (set-face-attribute 'some-face t :some-attribute 'unspecified)
> 
> does when
> 
> (set-face-attribute 'some-face nil :some-attribute 'unspecified)
> 
> has already been executed.

Do you understand what

  (set-face-attribute 'some-face t :some-attribute 'unspecified)

does differently from what

  (set-face-attribute 'some-face FRAME :some-attribute 'unspecified)

does?  I mean, besides the difference in FRAME argument value?

> And in fact that's how this bug report started: I asked whether
> anyone could come up with a scenario that would make the effect of
> that call with frame = t apparent.

See above: that's not what that part of the doc string attempts to
address.