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

[Eli Zaretskii <eliz@gnu.org>]

> Date: Wed, 31 Aug 2022 18:33:15 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 57499@debbugs.gnu.org
> 
> Just to be clear, I would be happier with the following docstring:
> 
> 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 only.
> 
> If FRAME is a frame, set the attributes only for that frame.  If FRAME is 
> nil, set the attributes for all existing frames, as well as the default 
> for new frames.  If FRAME is t, change the default for new frames only.
> 
> ARGS must come in pairs ATTRIBUTE VALUE.  ATTRIBUTE must be a valid face 
> attribute name.  All attributes can be set to `unspecified'; this fact is 
> not further mentioned below.  To set an attribute to `unspecified', the 
> symbol 'unspecified must be used.  Using nil may produce the same effect 
> in some cases, but is not guaranteed to work.

This is a step back, IMO.  It also repeats the same information more
than once, and confuses the nil vs unspecified issue for no good
reason.  How about the following instead:

  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 only.

  If FRAME is a frame, set the attributes only for that frame.  If FRAME is 
  nil, set the attributes for all existing frames, as well as the default 
  for new frames.  If FRAME is t, change the default for new frames only.

  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.

  All attributes can also be set to the special value `unspecified';
  this can be used to reset the value of ATTRIBUTE in a way that
  overrides any value defined by the face's spec in `defface'.

(Of course, after making such a change, we will again need to answer
questions how come using value of nil and FRAME = nil doesn't reset
the attribute, something that the current doc string avoids.  Oh
well.)