Subject: bug#27238: 24.5; Rename `face-spec-set-2' and give it a better doc string

From: Drew Adams <[email-masked]>
Date: Mon, 5 Jun 2017 08:55:06 -0700 (PDT)

> > > > (face-spec-set-2 TARGET-FACE
> > > >                  FRAME
> > > >                  (face-spec-choose
> > > >                    (get SOURCE-FACE 'face-defface-spec)
> > > >                    FRAME))
> > >
> > > Why can't you do this by calling a higher-level function?
> >
> > What higher-level function would you suggest?
> 
> face-spec-recalc and face-spec-set come to mind, for example.

I don't see how either of those can be used to do the same
thing.  If you think otherwise, please show how.

> > How would _you_ set one face (`fringe' or whatever), in
> > only a given frame, to the spec of another face (or to
> > a spec that isn't yet used for any face)?
> 
> I asked whether the higher-level functions can do the job.  If they
> cannot, please explain why, and please provide specific details about
> the difficulties.  Answering my question by another question doesn't
> help, since I'm sure I don't know enough about the job you wanted to
> do.

I don't know of any that could do the job, as I've said.
I described the situation with `face-spec-set', and said
that I tried to use it.  You can, I think, see that it
is not frame-specific.

The job to do is shown in the code I gave above.  And it is
described in the description I gave:

  "set one face (`fringe' or whatever), in only a given frame,
                                        ^^^^^^^^^^^^^^^^^^^^^
  to the spec of another face (or to a spec that isn't yet used
  for any face)"

I don't see a "higher-level function" that does that.  I don't
see any function that does, apart from `face-spec-2'.

> > But perhaps you have some other higher-level function in
> > mind to do this job?  If so, I wonder why `face-set-recalc'
> > doesn't use it, instead of `face-spec-set-2.
> 
> There are many documented functions that set face attributes in
> various forms and for various sets of frames.  My question is
> precisely whether any of them can do this job.

If I thought that any of them could then I would not have used
`face-spec-set-2'.  If you know of one that can do the job (see
above), please enlighten me.

> If not, perhaps we could extend one of them to support
> whatever you need to do.  That'd be an alternative to renaming
> face-spec-set-2 and making it public.

Perhaps.  Please consider letting, for example, `face-spec-set'
take an optional FRAME argument.

That would work for what I wanted to do.

But that would not directly help someone who wants to pass,
not a full face SPEC but the kind of non-spec "SPEC" arg that
`face-spec-set-2' accepts, which is the kind of thing that
`face-spec-choose' returns.  IOW, in a use case where what
you have to start with is not a face spec but a face-attribute
plist.  Not a big deal, but worth mentioning; `face-spec-set-2'
accepts such a plist directly.

In any case: (1) the name of `face-spec-2' is not very
meaningful, and more importantly, the doc is wrong and the
argument name "SPEC" is wrong.  The "SPEC" arg is the kind
of thing that `face-spec-choose' returns: a plist of face
attributes - it is not a face spec.

It's easy to see the difference when you check the definition
of something like `face-spec-match-p' (or other uses of
`face-spec-choose').  Its whole job is to just use
`face-spec-choose' to convert a full face SPEC to a list of
attributes for the given frame, so that it can then call
`face-attr-match-p'.

The doc for a function such as `face-spec-set-2' should, just
like that for `face-attr-match-p', refer to the "SPEC" argument
as ATTRS, not "SPEC", and it should say explicitly that it is
a plist of face attributes and their values.

Similar cleanup is called for in the code and comments of
`face-spec-set-recalc', where `face-spec-set-2' is used: The
variable "SPEC" there should be given another name, and the
comments should talk about it appropriately - it is not a
face spec.

A face-attribute plist is not a face spec.