bug#19217: 25.0.50; `C-M-x' (`eval-defun') on a `defface' that is not top-level

[Drew Adams <drew.adams@oracle.com>]

> Ah.  Yes, `defface' is like `defvar', in that if you defface a face
> twice, the second try does not overwrite the first.

`defvar' too should be fixed, for the same cases & reasons.  There
is no user-oriented reason that `C-M-x' should not re-evaluate the
sexp, redefining the variable or the face.

There can be implementation reasons (too hard to do, or whatever),
but I see no user reason.

And wrt the implementation, I see no reason why TRT cannot be done,
at least for common, simple use cases (e.g., point on the symbol
being redefined).

> So I now see what you're complaining about, and I think would too,
> if I were doing anything at all with faces.

> ... use C-M-x.  And for the latter, you need the opening paren
> at top level.

1. That evaluates more than the defface inside that opening paren.
   E.g., in the test case I gave, the opening top-level paren is
   for (when... ).

2. Even if that did the right thing, which does not (see #1), it
   is far too restrictive - far more restrictive than it needs to
   be.  See what I wrote above.

> If I were you, I'd hack something together for
> my own use based on what's in lisp-mode.el.
...
> You could probably fix this with a bit of advice (whether
> old-style or new-style ;-).

Thanks for the advice, but I prefer to file this bug report
and hope that Emacs gets fixed.  I've lived with it this way
for decades already.  It is for Emacs, not for me, that I file
the bug report.

> > (2) `C-M-x' with point inside a defface sexp should also work.
> > If the latter cannot easily be made to work with point anywhere
> > inside the sexp, then at least make it work with point on
> `defface'
> > or near it (e.g. at the same list level).
> 
> > > That `defface' be made a special case,
> 
> > If that's necessary, yes.  It should be made to work, whether that
> > means special-casing it or not.
> 
> Given how much of a special case faces already are, in terms of
> awkwardness and inflexibility, maybe that's not too much to ask for.
> But beware of what you're asking for - you might get it.  Then
> when you do C-M-x, expecting to evaluate a defun, you accidentally
> get a face defined instead, which you then can't get rid of.

Why can't I "get rid of it"?

Anyway, I can live with `C-M-x' redefining the stuff I use it on.
I will expect it to do that.  If I don't want to do that then I
can use `eval-region' or similar, which perform normal load-like
evaluation.

> > > I'm not sure how you're going to construe "closer", given that a
> > > list typically extends over many characters and when point is
> > > within it, that must count as distance zero.  Or something.
> 
> > It's trivial to determine whether point is at the same list level
> > as the `defface' symbol.
> 
> That would indeed be one way of doing it.

Thanks for taking a look, in any case.  Should someone decide to
fix this bug, it might be good to (a) take a user poll and (b)
provide a way (e.g. a user option) to return to the previous (i.e.,
current), lame behavior for those who prefer it.