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

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

[Drew Adams]

Enhancement request.  This has probably been requested before, but
here goes...

It is not uncommon to have a `defface' inside a condition such as (when
(> emacs-major-version 23)...).  Currently, `C-M-x' has no effect on
such a `defface' occurrence.  In order to have Emacs DTRT, I find myself
working around this limitation by temporarily moving the `defface' to
bol.

How about letting users redefine a `defface' with `C-M-x' even in this
case?

At least optionally.  For example, let a non-positive prefix arg do
this, and have edebug instrumentation done only for a non-negative
prefix arg.

Or maybe always (not optionally)?  Is there a good reason for doing this
only at top-level?

If the reason is that `eval-defun' cannot recognize the defun
(`defface') unless it is at top level, then surely there is some way to
make this more flexible.

In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
 of 2014-10-20 on LEG570
Bzr revision: 118168 rgm@gnu.org-20141020195941-icp42t8ttcnud09g
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --enable-checking=yes,glyphs CPPFLAGS=-DGLYPH_DEBUG=1'

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

[Alan Mackenzie]

Hi, Drew.

In article <mailman.14952.1417284925.1147.bug-gnu-emacs@gnu.org> you wrote:
> Enhancement request.  This has probably been requested before, but
> here goes...

> It is not uncommon to have a `defface' inside a condition such as (when
> (> emacs-major-version 23)...).  Currently, `C-M-x' has no effect on
> such a `defface' occurrence.  In order to have Emacs DTRT, I find myself
> working around this limitation by temporarily moving the `defface' to
> bol.

You can also move point to just after the closing ) and do C-x C-e
(`eval-last-sexp').

> How about letting users redefine a `defface' with `C-M-x' even in this
> case?

How is Emacs to determine which depth of parenthesis is to be considered
the opening one?  For example, if a defface is contained within a
defmacro, which one is to be executed on C-M-x?

> At least optionally.  For example, let a non-positive prefix arg do
> this, and have edebug instrumentation done only for a non-negative
> prefix arg.

> Or maybe always (not optionally)?  Is there a good reason for doing this
> only at top-level?

I think it is to make it unambiguous, which form is to be evaluated.

> If the reason is that `eval-defun' cannot recognize the defun
> (`defface') unless it is at top level, then surely there is some way to
> make this more flexible.

> In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
> of 2014-10-20 on LEG570
> Bzr revision: 118168 rgm@gnu.org-20141020195941-icp42t8ttcnud09g
> Windowing system distributor `Microsoft Corp.', version 6.1.7601
> Configured using:
> `configure --enable-checking=yes,glyphs CPPFLAGS=-DGLYPH_DEBUG=1'

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier]

> It is not uncommon to have a `defface' inside a condition such as (when
> (> emacs-major-version 23)...).

Obviously, I'm blindsided by looking mostly at bundled Elisp code, but
to get an idea of how common it might be, could you explain why you'd
want to wrap it in that way?
Also, pointers to code that do that would help.

> Currently, `C-M-x' has no effect on such a `defface' occurrence.

Yes, same problem for defvar/defcustom, of course.

> How about letting users redefine a `defface' with `C-M-x' even in this
> case?

I think I'd welcome a patch which fixes this well enough (including in
the case of C-u C-M-x).  The current way C-M-x handled defvar/defface is
hackish and ugly.  Extending this hack to code wrapped in a conditional
test would not be welcome.  But replacing the hack with something a bit
more generic would be nice.


        Stefan

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

[Drew Adams]

> > It is not uncommon to have a `defface' inside a condition such as
> > (when (> emacs-major-version 23)...).
> 
> Obviously, I'm blindsided by looking mostly at bundled Elisp code,
> but to get an idea of how common it might be, could you explain why
> you'd want to wrap it in that way?

I just did - when the defface (e.g. the given face itself) makes
sense only for certain Emacs versions.  It doesn't matter how
common it is - it is common enough.  There is no reason users should
not have a simple way to redefine a defface, defvar, or defcustom,
at top level or not.

This should not need to be justified by showing how commonly it is
needed.  Obviously this is only a nice-to-have; we've been getting
along without it for decades.  So what?

The current limitation to top-level is for what reason?  For users?
I'm guessing that the only answer is that this limitation simplifies
the *implementation*, not that it simplifies things for users.
30 years later, we ought to be able to do a little better (for users).

I suggested a simple improvement that can be made easily - let users
do it at least when point is on the defface etc. symbol.  That's a
limitation, but it at least gives you some way to easily redefine the
critter.

The point is that there is currently *no* way for a user to do it.
It's not about making things super general.  Just give us some easy
way to do it.  Let's not let the search for something ideal become
the enemy of making a simple improvement.

> > How about letting users redefine a `defface' with `C-M-x' even in
> > this case?
> 
> I think I'd welcome a patch which fixes this well enough
> (including in the case of C-u C-M-x).

I don't have a patch.  I too welcome a simple improvement here. 

> The current way C-M-x handled defvar/defface is hackish and ugly.

I assume you're talking about the implementation and not the way
users interact with Emacs using it.  The current user interaction
is simple and useful.

> Extending this hack to code wrapped in a conditional test would
> not be welcome.

Sounds like we're in for a few more decades of the same.

> But replacing the hack with something a bit more generic would
> be nice.

Provided we don't lose other functionality or convenience.
"Beware the generic rewrite improvement."  This should not be
a big deal.

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

[Stefan Monnier]

> I just did - when the defface (e.g. the given face itself) makes
> sense only for certain Emacs versions.

That just paraphrases in ambiguous English the perfectly precise Elisp
code you quoted.  So it's not what I'm after.

> It doesn't matter how common it is

It matters to me, obviously.

> - it is common enough.  There is no reason users should not have
> a simple way to redefine a defface, defvar, or defcustom, at top level
> or not.

Irrelevant.  Frequency of occurrence and typical contexts can give very
useful hints at what the best solution is.  The answer you quote already
makes it clear I agree it should be solved, so stop arguing this point,
you're just wasting people's time.

I stopped reading your rant here.


        Stefan

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

[Alan Mackenzie]

Hello again, Drew.

In article <mailman.15037.1417408221.1147.bug-gnu-emacs@gnu.org> you wrote:

> The current limitation to top-level is for what reason?  For users?

For both users and implementors.  "The top level construct containing
point (or the one following point)" is wonderfully consistent and clear
for all concerned.

> I'm guessing that the only answer is that this limitation simplifies
> the *implementation*, not that it simplifies things for users.
> 30 years later, we ought to be able to do a little better (for users).

I wasn't really aware of the special handling for `defvar', `defface',
etc. in C-M-x until just now.  In fact I thought C-M-x and C-x C-e did
the same thing.

As a suggestion, why not enhance C-x C-e also to handle these defining
constructs specially?

> I suggested a simple improvement that can be made easily - let users
> do it at least when point is on the defface etc. symbol.  That's a
> limitation, but it at least gives you some way to easily redefine the
> critter.

It's not rocket science, no, but it is still a bit tricky to do.

> The point is that there is currently *no* way for a user to do it.
> It's not about making things super general.  Just give us some easy
> way to do it.  Let's not let the search for something ideal become
> the enemy of making a simple improvement.

Again, I suggest enhancing C-x C-e.

> I don't have a patch.  I too welcome a simple improvement here. 

You don't have a patch _yet_.  You could write one.  I think there's a
good chance Stefan would accept it.

>> The current way C-M-x handled defvar/defface is hackish and ugly.

> I assume you're talking about the implementation and not the way
> users interact with Emacs using it.  The current user interaction
> is simple and useful.

Yes, the implementation is ugly.  Given how it breaks the rules about not
redefining defxxx's, I don't see how it can be other than ugly.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Drew Adams]

> You can also move point to just after the closing ) and do C-x C-e
> (`eval-last-sexp').

I too thought that was the case, but it does not seem to be.  I just
tried it, starting with emacs -Q in several Emacs versions (22, 24.4,
25 dev build).

> > How about letting users redefine a `defface' with `C-M-x' even in
> > this case?
> 
> How is Emacs to determine which depth of parenthesis is to be
> considered the opening one?  For example, if a defface is contained
> within a defmacro, which one is to be executed on C-M-x?

I really don't care about corner cases, if in fact there are any.

You could even require that point be on the symbol `defface' in the
sexp, for all I care.  Then it should be trivial to grab the `defface'
sexp (e.g., use `(list-at-point)').

The point is to have some way to reevaluate the defface sexp.  If
`C-x C-e' worked, that would be enough, but AFAICT it does not work.

> > Is there a good reason for doing this only at top-level?
> 
> I think it is to make it unambiguous, which form is to be evaluated.

Dunno how `C-x C-e' could be ambiguous wrt the sexp that precedes
point.  If the sexp preceding point is ambiguous then I think we're
probably in a world of trouble. ;-)

Coming back to `C-M-x': Then don't seek perfection.  Require that
point be closer to the list enclosing `defface' than to another
list when you try `C-M-x', in order for it to unambiguously pick
up the right sexp.

What am I missing?

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

[Alan Mackenzie]

Hello, Drew.

On Sat, Nov 29, 2014 at 12:08:14PM -0800, Drew Adams wrote:
> > You can also move point to just after the closing ) and do C-x C-e
> > (`eval-last-sexp').

> I too thought that was the case, but it does not seem to be.  I just
> tried it, starting with emacs -Q in several Emacs versions (22, 24.4,
> 25 dev build).

I've never had a problem with C-x C-e that I can remember, and that
includes several times with point after a random ) inside a defun.  What
happens when you do C-x C-e with point just after the `defface' form?

> > > How about letting users redefine a `defface' with `C-M-x' even in
> > > this case?

> > How is Emacs to determine which depth of parenthesis is to be
> > considered the opening one?  For example, if a defface is contained
> > within a defmacro, which one is to be executed on C-M-x?

> I really don't care about corner cases, if in fact there are any.

What exactly are you suggesting?  That `defface' be made a special case,
and that if any of the sequence of enclosing (s opens a `defface' form,
this should be the one chosen for execution?  Or should any defining
function count?  What is special about `defface'?

> You could even require that point be on the symbol `defface' in the
> sexp, for all I care.  Then it should be trivial to grab the `defface'
> sexp (e.g., use `(list-at-point)').

> The point is to have some way to reevaluate the defface sexp.  If
> `C-x C-e' worked, that would be enough, but AFAICT it does not work.

What happens when you try it?

> > > Is there a good reason for doing this only at top-level?

> > I think it is to make it unambiguous, which form is to be evaluated.

> Dunno how `C-x C-e' could be ambiguous wrt the sexp that precedes
> point.  If the sexp preceding point is ambiguous then I think we're
> probably in a world of trouble. ;-)

Neither C-M-x not C-x C-e are ambiguous.  The first evaluates the top
level form containing point, the second the sexp immediately preceding
point.

> Coming back to `C-M-x': Then don't seek perfection.  Require that
> point be closer to the list enclosing `defface' than to another
> list when you try `C-M-x', in order for it to unambiguously pick
> up the right sexp.

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.

> What am I missing?

A detailed clear specification of what you want.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Drew Adams]

> > > You can also move point to just after the closing ) and do C-x
> > > C-e (`eval-last-sexp').
> 
> > I too thought that was the case, but it does not seem to be.  I
> > just tried it, starting with emacs -Q in several Emacs versions
> > (22, 24.4, 25 dev build).
> 
> I've never had a problem with C-x C-e that I can remember, and that
> includes several times with point after a random ) inside a defun.
> What happens when you do C-x C-e with point just after the `defface'
> form?

As I said, nothing happens.

emacs -Q
Put this in *scratch*, and evaluate it:
(when t
  (defface foo '((((background dark)) (:foreground "#58DFFA4FFFFF"))
                 (t (:foreground "Firebrick")))
    "A face." :group 'help)
  )

`M-x customize-face' shows that it is defined as it should be.

Edit the definition, replacing "Firebrick" with "Blue".  Put point
just after the defface sexp and hit `C-x C-e'.  `M-x customize-face'
shows that no change was made.  Feel free to blow away the Customize
buffer completely, so you are sure that you're not just seeing what
was there before. ;-)

Edit the color to "Green".  Put point on the symbol `defface' and
hit `C-M-x' - likewise, still no change to the face.

> > > > How about letting users redefine a `defface' with `C-M-x' even
> > > > in this case?
> 
> > > How is Emacs to determine which depth of parenthesis is to be
> > > considered the opening one?  For example, if a defface is
> > > contained within a defmacro, which one is to be executed on C-M-x?
> 
> > I really don't care about corner cases, if in fact there are any.
> 
> What exactly are you suggesting?

See above.  (1) `C-x C-e' should work as you say it works.
(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.

Currently, `C-M-x' inside a defun that is not at top level does not
have the problem that defface has.  Repeat the above recipe, but
with (defun toto () "Toto." (forward-char)) in place of the defface
sexp.  Edit it after evalling, to use `backward-char', and use
`C-M-x' again.  Check the definition before and after editing, using
(symbol-function 'toto).  No problem.  Not so, for defface.

> and that if any of the sequence of enclosing (s opens a `defface'
> form, this should be the one chosen for execution?  Or should any
> defining function count?  What is special about `defface'?

No idea what you are going on about; sorry.

> > You could even require that point be on the symbol `defface' in
> > the sexp, for all I care.  Then it should be trivial to grab the
> > `defface' sexp (e.g., use `(list-at-point)').
> 
> > The point is to have some way to reevaluate the defface sexp.  If
> > `C-x C-e' worked, that would be enough, but AFAICT it does not
> > work.
> 
> What happens when you try it?

See above.  100% reproducible from emacs -Q.  What happens when
you try it? ;-)

> > > > Is there a good reason for doing this only at top-level?
> 
> > > I think it is to make it unambiguous, which form is to be
> > > evaluated.
> 
> > Dunno how `C-x C-e' could be ambiguous wrt the sexp that precedes
> > point.  If the sexp preceding point is ambiguous then I think
> > we're probably in a world of trouble. ;-)
> 
> Neither C-M-x not C-x C-e are ambiguous.  The first evaluates the
> top level form containing point, the second the sexp immediately
> preceding point.

They should.  But they do not, in the case of defface.  See above.
Try it.

> > Coming back to `C-M-x': Then don't seek perfection.  Require that
> > point be closer to the list enclosing `defface' than to another
> > list when you try `C-M-x', in order for it to unambiguously pick
> > up the right sexp.
> 
> 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.

> > What am I missing?
> 
> A detailed clear specification of what you want.

I don't think I'm (or you are) missing that.  Have you tried it?

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

[Alan Mackenzie]

Hello, Drew.

On Sun, Nov 30, 2014 at 12:51:59PM -0800, Drew Adams wrote:
> > > > You can also move point to just after the closing ) and do C-x
> > > > C-e (`eval-last-sexp').

> > > I too thought that was the case, but it does not seem to be.  I
> > > just tried it, starting with emacs -Q in several Emacs versions
> > > (22, 24.4, 25 dev build).

> > I've never had a problem with C-x C-e that I can remember, and that
> > includes several times with point after a random ) inside a defun.
> > What happens when you do C-x C-e with point just after the `defface'
> > form?

> As I said, nothing happens.

Ah.  Yes, `defface' is like `defvar', in that if you defface a face
twice, the second try does not overwrite the first.  I had to look at the
fine source to see this, the documentation being somewhat suboptimal.

There's presumably some way of deleting a face, but I'm dashed if I can
find it in the info pages.  Something like `makunbound', but for faces.
I suspect that this will also be only to be found by perusal of the fine
source, possibly the C source.  <A bit later>  I've had a look at
xfaces.c, and it would seem there is no way to delete a face.  Faces are
simply stored in an array of Lisp_object's, starting at element 0, and
counting up from there each time you add a new face.

It was also said, somewhere, that C-M-x takes special action for a
variable, customisable something-or-other, or face.  So how does it do
this?  Well, a bit of digging around, and the answer's in `eval-defun-1'
in lisp-mode.el.

So I now see what you're complaining about, and I think would too, if I
were doing anything at all with faces.  Having defined a face with
`defface', the _ONLY_ way to tweak it at the source code level, is either
to restart Emacs or use C-M-x.  And for the latter, you need the opening
paren at top level.  If I were you, I'd hack something together for my
own use based on what's in lisp-mode.el.

Phew!

> emacs -Q
> Put this in *scratch*, and evaluate it:
> (when t
>   (defface foo '((((background dark)) (:foreground "#58DFFA4FFFFF"))
>                  (t (:foreground "Firebrick")))
>     "A face." :group 'help)
>   )

> `M-x customize-face' shows that it is defined as it should be.

> > What exactly are you suggesting?

> See above.  (1) `C-x C-e' should work as you say it works.

You could probably fix this with a bit of advice (whether old-style or
new-style ;-).

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

[ .... ]

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

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Drew Adams]

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

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

[Alan Mackenzie]

Hi, Drew.

On Sun, Nov 30, 2014 at 12:51:59PM -0800, Drew Adams wrote:
> > > > You can also move point to just after the closing ) and do C-x
> > > > C-e (`eval-last-sexp').

> > > I too thought that was the case, but it does not seem to be.  I
> > > just tried it, starting with emacs -Q in several Emacs versions
> > > (22, 24.4, 25 dev build).

> > I've never had a problem with C-x C-e that I can remember, and that
> > includes several times with point after a random ) inside a defun.
> > What happens when you do C-x C-e with point just after the `defface'
> > form?

> As I said, nothing happens.

OK.  I've hacked the following patch together, which wasn't actually all
that difficult.  I've tested it out on a defvar, and it works there.  It
gives C-x C-e the same special handling that C-M-x has for defvar's,
defcustom's, defface's, etc.  It's incomplete (no documentation has been
changed), but it might do what you need.  Please try it out.



diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 2be81a5..830ae2b 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -811,10 +811,11 @@ and level of lists, and include additional formats for integers
   (let ((standard-output (if eval-last-sexp-arg-internal (current-buffer) t)))
     ;; Setup the lexical environment if lexical-binding is enabled.
     (elisp--eval-last-sexp-print-value
-     (eval (eval-sexp-add-defvars (elisp--preceding-sexp)) lexical-binding)
+     (eval (eval-sexp-add-defvars
+	    (elisp--eval-defun-1 (macroexpand (elisp--preceding-sexp))))
+	   lexical-binding)
      eval-last-sexp-arg-internal)))
 
-
 (defun elisp--eval-last-sexp-print-value (value &optional eval-last-sexp-arg-internal)
   (let ((unabbreviated (let ((print-length nil) (print-level nil))
 			 (prin1-to-string value)))



-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Drew Adams]

Hi Alan,

> > > What happens when you do C-x C-e with point just after the
> > > `defface' form?
> 
> > As I said, nothing happens.
> 
> OK.  I've hacked the following patch together, which wasn't actually
> all that difficult.  I've tested it out on a defvar, and it works there.
> It gives C-x C-e the same special handling that C-M-x has for defvar's,
> defcustom's, defface's, etc.  It's incomplete (no documentation has
> been changed), but it might do what you need.  Please try it out.

Sorry for the delay.  I tried it, for both defface and defvar, using
this:

(when t
  (defface foo '((((background dark)) (:foreground "#58DFFA4FFFFF"))
                 (t (:foreground "Firebrick")))
    "A face." :group 'help);
  )

(when t
  (defvar toto 42
    "Forty-two.");
  )

After evaling the sexps and then modifying the values
(e.g. Firebrick -> Blue, 42 -> 56), I put point before the `;'
and tried C-x C-e.

Starting from emacs -Q, it works, for both face & var.  With my
setup, however, it does not work (no change from before).  Dunno why.
Tried it with `lexical-binding' t and nil - neither worked.

I'll bisect my setup when I get a moment, to try to find out why it
doesn't work for me.

Thanks for working on this.

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

[Drew Adams]

Hi Alan,

> Starting from emacs -Q, it works, for both face & var.  With my
> setup, however, it does not work (no change from before).  Dunno
> why.  Tried it with `lexical-binding' t and nil - neither worked.
> 
> I'll bisect my setup when I get a moment, to try to find out why it
> doesn't work for me.
> 
> Thanks for working on this.

I found the explanation.  I bind `C-x C-e' to `pp-eval-last-sexp',
not to `eval-last-sexp'.  Apparently the latter got considerably
redefined after Emacs 24.4, so that `pp-eval-last-sexp' is no
longer parallel with `eval-last-sexp' - the code is very different.

Your fix seems good for `eval-last-sexp'.  But I would like to
also see a similar fix for `pp-eval-last-sexp'.  Could you
perhaps take a look at fixing that as well?

I filed a separate bug (#19362) for bringing `pp.el' into line
with the changes that were made in moving from `lisp-mode.el'
to `elisp-mode.el'.  I'm not sure about all that might be
involved for such a fix, as I don't know just what has changed.
So that bug report is worded vaguely.

But I would at least like to see `pp-eval-last-sexp' updated
similarly to how `eval-last-sexp' was (for Emacs-Lisp buffers).
A quick glance suggests that the change involves respecting
`lexical-binding', but it might be more or other than that.

Could you perhaps take a look?  I would like to be able to
get both (a) any advantages brought to `eval-last-sexp' by
the Emacs 25 changes to it and (b) the advantage of your
fix to it (this bug).  But I also want to be able to continue
using `pp-eval-last-sexp' instead of `eval-last-sexp'.
I just want the former to enjoy the same advantages as latter.

I have my own enhancement to `pp-eval-expression' (which is
used by `pp-eval-last-sexp'), but the changes I make to that
are probably orthogonal and I will be able to make them to
whatever updated definition of `pp-eval-last-sexp' there is.

---

FYI, the changes I make to `pp-eval-expression' are these:

1. Read with completion using `pp-read-expression-map'.
2. Progress message added.
3. Added optional arg and insertion behavior.
4. Respect `pp-eval-expression-print-length',
   `pp-eval-expression-print-level', and
   `eval-expression-debug-on-error'.

#1 means bindings for `lisp-indent-line', `lisp-complete-symbol',
   `indent-sexp', `eval-defun', and `indent-pp-sexp'.

#3 means this:
With a prefix arg, insert the value into the current buffer at point.
 With a negative prefix arg, if the value is a string then insert it
 into the buffer without double-quotes (`"').

#4 means you can use different values from the same
options without the prefix `pp-'.

---

FYI, I do this, wrt keys:
(substitute-key-definition 'eval-last-sexp
                           'pp-eval-last-sexp global-map)
(substitute-key-definition 'eval-expression
                           'pp-eval-expression global-map))

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

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> OK.  I've hacked the following patch together, which wasn't actually all
> that difficult.  I've tested it out on a defvar, and it works there.  It
> gives C-x C-e the same special handling that C-M-x has for defvar's,
> defcustom's, defface's, etc.  It's incomplete (no documentation has been
> changed), but it might do what you need.  Please try it out.

The patch no longer applies to Emacs 28, so I've respun it:

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 72b94a57b4..7682e099f5 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1190,7 +1190,8 @@ elisp--eval-last-sexp
     ;; Setup the lexical environment if lexical-binding is enabled.
     (elisp--eval-last-sexp-print-value
      (eval (macroexpand-all
-            (eval-sexp-add-defvars (elisp--preceding-sexp)))
+            (eval-sexp-add-defvars
+             (elisp--eval-defun-1 (macroexpand (elisp--preceding-sexp)))))
            lexical-binding)
      (if insert-value (current-buffer) t) no-truncate char-print-limit)))
 
To recap, this allows `C-x C-e' on defvar/defface to work the same as
`C-M-x' on those (when they are top-level).

That these two work differently is a source of frustration, so I think
it makes conceptual sense, but I'm not sure what the repercussions would
be otherwise...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Drew Adams]

What about `pp-eval-last-sexp'?
Could we get that updated similarly?

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19217#44

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

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> What about `pp-eval-last-sexp'?
> Could we get that updated similarly?

Sure, if we decide to alter `eval-last-sexp' in this way.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> To recap, this allows `C-x C-e' on defvar/defface to work the same as
> `C-M-x' on those (when they are top-level).
>
> That these two work differently is a source of frustration, so I think
> it makes conceptual sense, but I'm not sure what the repercussions would
> be otherwise...

There were no protests, so I've now gone ahead and done this.  This may
perhaps be controversial, but I've never understood why `C-x C-e'
shouldn't do "the obvious thing" on defvar (etc) forms, so I've done it
anyway.

However, if this somehow gets in the way of people's way of working, it
could be reversed, I guess.  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Drew Adams]

> > To recap, this allows `C-x C-e' on defvar/defface to work the same as
> > `C-M-x' on those (when they are top-level).
> >
> > That these two work differently is a source of frustration, so I think
> > it makes conceptual sense, but I'm not sure what the repercussions would
> > be otherwise...
> 
> There were no protests, so I've now gone ahead and done this.  This may
> perhaps be controversial, but I've never understood why `C-x C-e'
> shouldn't do "the obvious thing" on defvar (etc) forms, so I've done it
> anyway.
> 
> However, if this somehow gets in the way of people's way of working, it
> could be reversed, I guess.

Thanks for doing this.
But did you also fix `pp-eval-last-sexp'?

You said this:

> Saturday, September 19, 2020 7:11 AM
> > What about `pp-eval-last-sexp'?
> > Could we get that updated similarly?
> 
> Sure, if we decide to alter `eval-last-sexp' in this way.

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

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> But did you also fix `pp-eval-last-sexp'?

Yes.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Drew Adams]

Great! Thank you.

> > But did you also fix `pp-eval-last-sexp'?
> 
> Yes.