bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Drew Adams]

Library `elisp-mode.el' was added after Emacs 24.4.  Prior to that,
functions such as `pp-eval-last-sexp' were aligned with the non-pp
sister functions from `lisp-mode.el', such as `eval-last-sexp'.

This is no longer the case.  Please bring `pp.el' up to be parallel with
the behavior of `elisp-mode.el', just as it was parallel with the same
and similar code that was previously in `lisp-mode.el'.

In particular, `pp-eval-last-sexp' had the same evaluation behavior as
`eval-lisp-mode'.  The addition of pretty-printing was pretty much the
only difference.  The two no longer correspond.  Please fix this
disparity.



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#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Lars Ingebrigtsen]

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

> Library `elisp-mode.el' was added after Emacs 24.4.  Prior to that,
> functions such as `pp-eval-last-sexp' were aligned with the non-pp
> sister functions from `lisp-mode.el', such as `eval-last-sexp'.
>
> This is no longer the case.  Please bring `pp.el' up to be parallel with
> the behavior of `elisp-mode.el', just as it was parallel with the same
> and similar code that was previously in `lisp-mode.el'.
>
> In particular, `pp-eval-last-sexp' had the same evaluation behavior as
> `eval-lisp-mode'.  The addition of pretty-printing was pretty much the
> only difference.  The two no longer correspond.  Please fix this
> disparity.

What are the differences you think should be fixed?

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

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Drew Adams]

> > Library `elisp-mode.el' was added after Emacs 24.4.  Prior to that,
> > functions such as `pp-eval-last-sexp' were aligned with the non-pp
> > sister functions from `lisp-mode.el', such as `eval-last-sexp'.
> >
> > This is no longer the case.  Please bring `pp.el' up to be parallel with
> > the behavior of `elisp-mode.el', just as it was parallel with the same
> > and similar code that was previously in `lisp-mode.el'.
> >
> > In particular, `pp-eval-last-sexp' had the same evaluation behavior as
> > `eval-lisp-mode'.  The addition of pretty-printing was pretty much the
> > only difference.  The two no longer correspond.  Please fix this
> > disparity.
> 
> What are the differences you think should be fixed?

Should be aligned as it was before.  Someone evolved the one
without evolving the other in tandem.

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Lars Ingebrigtsen]

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

>> > Library `elisp-mode.el' was added after Emacs 24.4.  Prior to that,
>> > functions such as `pp-eval-last-sexp' were aligned with the non-pp
>> > sister functions from `lisp-mode.el', such as `eval-last-sexp'.
>> >
>> > This is no longer the case.  Please bring `pp.el' up to be parallel with
>> > the behavior of `elisp-mode.el', just as it was parallel with the same
>> > and similar code that was previously in `lisp-mode.el'.
>> >
>> > In particular, `pp-eval-last-sexp' had the same evaluation behavior as
>> > `eval-lisp-mode'.  The addition of pretty-printing was pretty much the
>> > only difference.  The two no longer correspond.  Please fix this
>> > disparity.
>> 
>> What are the differences you think should be fixed?
>
> Should be aligned as it was before.  Someone evolved the one
> without evolving the other in tandem.

Do you have a list of things that you think should be aligned?

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

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Drew Adams]

> Do you have a list of things that you think should be aligned?

No, but diff of the source code should show it. ;-)

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Lars Ingebrigtsen]

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

>> Do you have a list of things that you think should be aligned?
>
> No, but diff of the source code should show it. ;-)

I'm afraid that if you don't have anything specific you want to have
fixed, there won't be any more progress happening in this bug report.

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

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[npostavs]

tags 19362 moreinfo
quit

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

>> Do you have a list of things that you think should be aligned?
>
> No, but diff of the source code should show it. ;-)

I tried diffing pp.el and elisp-mode.el, but it was unenlightening.
Like this bug report.

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Drew Adams]

> >> Do you have a list of things that you think should be aligned?
> >
> > No, but diff of the source code should show it. ;-)
> 
> I tried diffing pp.el and elisp-mode.el, but it was unenlightening.
> Like this bug report.

Yes, well, it's quite difficult.  Some of the code from lisp-mode.el
was moved to elisp-mode.el.  And then it was modified, in some cases
radically.  Dunno why, except that some of the changes seem to have
involved integrating eldoc.  But most of the changes presumably do not
concern this bug, which is only about the part of lisp-mode.el that
had functions that correspond to pp.el functions.)

===> The starting point is `eval-last-sexp', which used to correspond
directly with `pp-eval-last-sexp'.  The former was (presumably)
improved, but the latter was not modified similarly.

Some functions were renamed to add the prefix `elisp-' or `elisp--'
(e.g. `elisp--preceding-sexp', `elisp--eval-last-sexp',
`elisp--eval-last-sexp-print-value', `elisp--eval-last-sexp-fake-value',
`elisp--eval-defun-1', `elisp--eval-defun'), so knowing that can help.
In some cases they were just renamed.  In other cases the code was
changed quite a bit.  But again, this bug is only about the pp-like
code.

This is not something you will understand in 5 minutes.  It likely
requires understanding the changes that were made to `eval-last-sexp'
and its supporting functions, and then doing the right thing (not
necessarily exactly the same thing) for `pp-eval-last-sexp'.  The
pp.el code was slightly different from the Emacs 24.4 (and prior)
lisp-mode.el code (different helper functions, and one does not
involve pretty-printing), but they generally mirror one another.
Using their former correspondence as a guide should help.

I cannot help more than this.  It is unfortunate that the person
who changed the non-pp version did not think to act similarly for
the pp version.  I discovered it late, myself.  Once things are
properly understood, perhaps the code fix will be minor.  Dunno.

With luck, you will find and understand the changes made to
`eval-last-sexp' as a straightforward improvement that do not
involve all of the other changes from lisp-mode.el code to
elisp-mode.el code.  With luck, perhaps the person(s) who made
the changes to the non-pp code can look at this bug.  No doubt
that would be easiest.

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Noam Postavsky]

On Thu, Jun 30, 2016 at 11:07 PM, Drew Adams <drew.adams@oracle.com> wrote:
>> >> Do you have a list of things that you think should be aligned?
>> >
>> > No, but diff of the source code should show it. ;-)
>>
>> I tried diffing pp.el and elisp-mode.el, but it was unenlightening.
>> Like this bug report.
>
> Yes, well, it's quite difficult.  Some of the code from lisp-mode.el
> was moved to elisp-mode.el.  And then it was modified, in some cases
> radically.  Dunno why, except that some of the changes seem to have
> involved integrating eldoc.  But most of the changes presumably do not
> concern this bug, which is only about the part of lisp-mode.el that
> had functions that correspond to pp.el functions.)
>
> ===> The starting point is `eval-last-sexp', which used to correspond
> directly with `pp-eval-last-sexp'.  The former was (presumably)
> improved, but the latter was not modified similarly.
>
> Some functions were renamed to add the prefix `elisp-' or `elisp--'
> (e.g. `elisp--preceding-sexp', `elisp--eval-last-sexp',
> `elisp--eval-last-sexp-print-value', `elisp--eval-last-sexp-fake-value',
> `elisp--eval-defun-1', `elisp--eval-defun'), so knowing that can help.
> In some cases they were just renamed.  In other cases the code was
> changed quite a bit.  But again, this bug is only about the pp-like
> code.
>
> This is not something you will understand in 5 minutes.  It likely
> requires understanding the changes that were made to `eval-last-sexp'
> and its supporting functions, and then doing the right thing (not
> necessarily exactly the same thing) for `pp-eval-last-sexp'.  The
> pp.el code was slightly different from the Emacs 24.4 (and prior)
> lisp-mode.el code (different helper functions, and one does not
> involve pretty-printing), but they generally mirror one another.
> Using their former correspondence as a guide should help.
>
> I cannot help more than this.  It is unfortunate that the person
> who changed the non-pp version did not think to act similarly for
> the pp version.  I discovered it late, myself.  Once things are
> properly understood, perhaps the code fix will be minor.  Dunno.

But do you know of any concrete cases where there is a difference in
behaviour? Or is this report just about code duplication (or lack
thereof)?

I found #10495 "pp-eval-last-sexp doesn't work on a `symbol' in
quotes", but that was reported against 24.0.92, so perhaps these
functions were in fact never "aligned"?

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Drew Adams]

> But do you know of any concrete cases where there is a difference in
> behaviour? Or is this report just about code duplication (or lack
> thereof)?

1. I don't know about concrete cases; sorry.

2. This report is an enhancement request; it doesn't report a bug.

In the past, `eval-last-sexp' and `pp-eval-last-sexp' did about the
same thing, apart from the pretty-printing part (which the latter
farms out to another function).  My guess is that _improvements_
were made to the former case (only).  Just what those improvements
were and why they were made I don't know.

> I found #10495 "pp-eval-last-sexp doesn't work on a `symbol' in
> quotes", but that was reported against 24.0.92, so perhaps these
> functions were in fact never "aligned"?

The functions used to be mostly "aligned", but it's possible that
the difference Michael points out in #10495 was present.  I don't
know.

In any case, I was not really referring to the interactive behavior
but to the code/behavior after the sexp has been determined.  In
the case of `eval-last-sexp' I guess that means the code other
than `elisp--preceding-sexp'.  I'm interested in both, but I don't
think I was paying attention to differences that are covered by the
`elisp--preceding-sexp' code.

Michael's bug is all about extending what `elisp--preceding-sexp'
does to pp.el.  It could perhaps be a good start, in terms of
realignment.  On the other hand, that behavior seems to be overly
DWIM, making heuristic assumptions about what sexp you _really_
wanted to evaluate.  I'm not sure that's always such a good thing.
I'd probably rather have that DWIM be a user choice (option).
`pp-eval-last-sexp' is much simpler.

In any case, what `elisp--preceding-sexp' is/does is not really
what I had in mind about the code divergence, as you rightfully
observed.  So I guess this bug report is somewhat complementary
to Michael's report.

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Noam Postavsky]

On Wed, Jul 6, 2016 at 6:35 PM, Drew Adams <drew.adams@oracle.com> wrote:
>> But do you know of any concrete cases where there is a difference in
>> behaviour? Or is this report just about code duplication (or lack
>> thereof)?
>
> 1. I don't know about concrete cases; sorry.
>
> 2. This report is an enhancement request; it doesn't report a bug.
>
> In the past, `eval-last-sexp' and `pp-eval-last-sexp' did about the
> same thing, apart from the pretty-printing part (which the latter
> farms out to another function).

So you're not talking about the difference between pp-to-string vs
elisp--eval-last-sexp-print-value.


> My guess is that _improvements_
> were made to the former case (only).  Just what those improvements
> were and why they were made I don't know.
[...]
> In any case, I was not really referring to the interactive behavior
> but to the code/behavior after the sexp has been determined.  In
> the case of `eval-last-sexp' I guess that means the code other
> than `elisp--preceding-sexp'.

And you're not talking about the difference between (pp-last-sexp) vs
(eval-sexp-add-defvars (elisp--preceding-sexp)).

What's left? They both call eval in the middle. eval-last-sexp honours
eval-expression-debug-on-error while pp-eval-last-sexp does not (this
was the case for the old lisp-mode.el code in 24.3 as well). Other
than that I don't see anything of significance.

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Drew Adams]

> > In the past, `eval-last-sexp' and `pp-eval-last-sexp' did about the
> > same thing, apart from the pretty-printing part (which the latter
> > farms out to another function).
> 
> So you're not talking about the difference between pp-to-string vs
> elisp--eval-last-sexp-print-value.
> 
> > My guess is that _improvements_
> > were made to the former case (only).  Just what those improvements
> > were and why they were made I don't know.
> [...]
> > In any case, I was not really referring to the interactive behavior
> > but to the code/behavior after the sexp has been determined.  In
> > the case of `eval-last-sexp' I guess that means the code other
> > than `elisp--preceding-sexp'.
> 
> And you're not talking about the difference between (pp-last-sexp) vs
> (eval-sexp-add-defvars (elisp--preceding-sexp)).
> 
> What's left? They both call eval in the middle. eval-last-sexp honours
> eval-expression-debug-on-error while pp-eval-last-sexp does not (this
> was the case for the old lisp-mode.el code in 24.3 as well). Other
> than that I don't see anything of significance.

Sorry, but I have no more time to devote to this.  I pointed to a
time where the code was more or less the same between the two, and
to a time where it had been changed to be really quite different.

It seemed (and still seems) clear to me that the non-pp version was
altered considerably - probably improving something, or adapting to
some other change (lexical binding, perhaps?), and the pp version
was not altered similarly.  My guess is that the pp version was
considered less important or was simply overlooked/ignored.

I think it deserves a similar look.  If no one wants to do that,
so be it.

If you feel you've taken a careful look and understand what changed
and why, and that none of the changes can be usefully extended to
pp, fine.  I'm not looking at this anymore, so you'll get no argument
from me.  I think I've said all I want to say about this.  Thanks for
having taken a look.

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 709 bytes --]

tags 19362 notabug
close 19362
quit

Drew Adams <drew.adams@oracle.com> writes:
>
> Sorry, but I have no more time to devote to this.  I pointed to a
> time where the code was more or less the same between the two, and
> to a time where it had been changed to be really quite different.

Well, we could have all saved some time if you had taken your own advice
to diff the code; not pp.el vs elisp-mode.el, but emacs-24.5's
lisp-mode.el vs the new elisp-mode.el.  You can see in the attached diff
(exerpted to leave only the relevant functions) that the only changes
are renaming of functions, and some minor refactoring in
elisp--preceding-sexp (it now handles ‘foo’ as well as `foo').


[-- Attachment #2: diff excerpts --]
[-- Type: text/x-diff, Size: 8838 bytes --]

--- /home/npostavs/src/emacs/emacs-24.5/lisp/emacs-lisp/lisp-mode.el	2016-05-21 14:56:43.119505998 -0400
+++ /home/npostavs/src/emacs/emacs-25/lisp/progmodes/elisp-mode.el	2016-06-27 00:32:51.740892449 -0400
@@ -879,14 +970,14 @@
 
 
 (defun last-sexp-setup-props (beg end value alt1 alt2)
-  "Set up text properties for the output of `eval-last-sexp-1'.
+  "Set up text properties for the output of `elisp--eval-last-sexp'.
 BEG and END are the start and end of the output in current-buffer.
 VALUE is the Lisp value printed, ALT1 and ALT2 are strings for the
 alternative printed representations that can be displayed."
   (let ((map (make-sparse-keymap)))
-    (define-key map "\C-m" 'last-sexp-toggle-display)
+    (define-key map "\C-m" 'elisp-last-sexp-toggle-display)
     (define-key map [down-mouse-2] 'mouse-set-point)
-    (define-key map [mouse-2] 'last-sexp-toggle-display)
+    (define-key map [mouse-2] 'elisp-last-sexp-toggle-display)
     (add-text-properties
      beg end
      `(printed-value (,value ,alt1 ,alt2)
@@ -897,7 +988,7 @@
 						printed-value)))))
 
 
-(defun last-sexp-toggle-display (&optional _arg)
+(defun elisp-last-sexp-toggle-display (&optional _arg)
   "Toggle between abbreviated and unabbreviated printed representations."
   (interactive "P")
   (save-restriction
@@ -920,7 +1011,7 @@
 				 (nth 1 value))
 	  (goto-char (min (point-max) point)))))))
 
-(defun prin1-char (char)
+(defun prin1-char (char)                ;FIXME: Move it, e.g. to simple.el.
   "Return a string representing CHAR as a character rather than as an integer.
 If CHAR is not a character, return nil."
   (and (integerp char)
@@ -956,19 +1047,20 @@
 	      (= (car (read-from-string string)) char)
 	      string))))
 
-
-(defun preceding-sexp ()
+(defun elisp--preceding-sexp ()
   "Return sexp before the point."
   (let ((opoint (point))
-	ignore-quotes
+	(left-quote ?‘)
 	expr)
     (save-excursion
       (with-syntax-table emacs-lisp-mode-syntax-table
-	;; If this sexp appears to be enclosed in `...'
+	;; If this sexp appears to be enclosed in `...' or ‘...’
 	;; then ignore the surrounding quotes.
-	(setq ignore-quotes
-	      (or (eq (following-char) ?\')
-		  (eq (preceding-char) ?\')))
+	(cond ((eq (preceding-char) ?’)
+	       (progn (forward-char -1) (setq opoint (point))))
+	      ((or (eq (following-char) ?\')
+		   (eq (preceding-char) ?\'))
+	       (setq left-quote ?\`)))
 	(forward-sexp -1)
 	;; If we were after `?\e' (or similar case),
 	;; use the whole thing, not just the `e'.
@@ -992,43 +1084,37 @@
 	      (forward-sexp -1))))
 
 	(save-restriction
-	  ;; vladimir@cs.ualberta.ca 30-Jul-1997: skip ` in
-	  ;; `variable' so that the value is returned, not the
-	  ;; name
-	  (if (and ignore-quotes
-		   (eq (following-char) ?`))
+	  (if (eq (following-char) left-quote)
+              ;; vladimir@cs.ualberta.ca 30-Jul-1997: Skip ` in `variable' so
+              ;; that the value is returned, not the name.
 	      (forward-char))
+          (when (looking-at ",@?") (goto-char (match-end 0)))
 	  (narrow-to-region (point-min) opoint)
 	  (setq expr (read (current-buffer)))
-	  ;; If it's an (interactive ...) form, it's more
-	  ;; useful to show how an interactive call would
-	  ;; use it.
-	  (and (consp expr)
-	       (eq (car expr) 'interactive)
+          ;; If it's an (interactive ...) form, it's more useful to show how an
+          ;; interactive call would use it.
+          ;; FIXME: Is it really the right place for this?
+          (when (eq (car-safe expr) 'interactive)
 	       (setq expr
-		     (list 'call-interactively
-			   (list 'quote
-				 (list 'lambda
-				       '(&rest args)
-				       expr
-				       'args)))))
+                  `(call-interactively
+                    (lambda (&rest args) ,expr args))))
 	  expr)))))
+(define-obsolete-function-alias 'preceding-sexp 'elisp--preceding-sexp "25.1")
 
-
-(defun eval-last-sexp-1 (eval-last-sexp-arg-internal)
+(defun elisp--eval-last-sexp (eval-last-sexp-arg-internal)
   "Evaluate sexp before point; print value in the echo area.
-With argument, print output into current buffer.
-With a zero prefix arg, print output with no limit on the length
-and level of lists, and include additional formats for integers
-\(octal, hexadecimal, and character)."
+If EVAL-LAST-SEXP-ARG-INTERNAL is non-nil, print output into
+current buffer.  If EVAL-LAST-SEXP-ARG-INTERNAL is `0', print
+output with no limit on the length and level of lists, and
+include additional formats for integers \(octal, hexadecimal, and
+character)."
   (let ((standard-output (if eval-last-sexp-arg-internal (current-buffer) t)))
     ;; Setup the lexical environment if lexical-binding is enabled.
-    (eval-last-sexp-print-value
-     (eval (eval-sexp-add-defvars (preceding-sexp)) lexical-binding)
+    (elisp--eval-last-sexp-print-value
+     (eval (eval-sexp-add-defvars (elisp--preceding-sexp)) lexical-binding)
      eval-last-sexp-arg-internal)))
 
-
-(defun eval-last-sexp-print-value (value &optional 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)))
 	(print-length (and (not (zerop (prefix-numeric-value
@@ -1055,7 +1141,7 @@
 	))))
 
 
-(defvar eval-last-sexp-fake-value (make-symbol "t"))
+(defvar elisp--eval-last-sexp-fake-value (make-symbol "t"))
 
 (defun eval-sexp-add-defvars (exp &optional pos)
   "Prepend EXP with all the `defvar's that precede it in the buffer.
@@ -1092,16 +1178,16 @@
 this command arranges for all errors to enter the debugger."
   (interactive "P")
   (if (null eval-expression-debug-on-error)
-      (eval-last-sexp-1 eval-last-sexp-arg-internal)
+      (elisp--eval-last-sexp eval-last-sexp-arg-internal)
     (let ((value
-	   (let ((debug-on-error eval-last-sexp-fake-value))
-	     (cons (eval-last-sexp-1 eval-last-sexp-arg-internal)
+	   (let ((debug-on-error elisp--eval-last-sexp-fake-value))
+	     (cons (elisp--eval-last-sexp eval-last-sexp-arg-internal)
 		   debug-on-error))))
-      (unless (eq (cdr value) eval-last-sexp-fake-value)
+      (unless (eq (cdr value) elisp--eval-last-sexp-fake-value)
 	(setq debug-on-error (cdr value)))
       (car value))))
 
-(defun eval-defun-1 (form)
+(defun elisp--eval-defun-1 (form)
   "Treat some expressions specially.
 Reset the `defvar' and `defcustom' variables to the initial value.
 \(For `defcustom', use the :set function if there is one.)
@@ -1144,17 +1230,17 @@
 	   (put face-symbol 'face-override-spec nil))
 	 form)
 	((eq (car form) 'progn)
-	 (cons 'progn (mapcar 'eval-defun-1 (cdr form))))
+	 (cons 'progn (mapcar #'elisp--eval-defun-1 (cdr form))))
 	(t form)))
 
-(defun eval-defun-2 ()
+(defun elisp--eval-defun ()
   "Evaluate defun that point is in or before.
 The value is displayed in the echo area.
 If the current defun is actually a call to `defvar',
 then reset the variable using the initial value expression
 even if the variable already has some other value.
 \(Normally `defvar' does not change the variable's value
-if it already has a value.\)
+if it already has a value.)
 
 Return the result of evaluation."
   ;; FIXME: the print-length/level bindings should only be applied while
@@ -1177,7 +1263,7 @@
           (setq end (point)))
         ;; Alter the form if necessary.
         (let ((form (eval-sexp-add-defvars
-                     (eval-defun-1 (macroexpand form)))))
+                     (elisp--eval-defun-1 (macroexpand form)))))
           (eval-region beg end standard-output
                        (lambda (_ignore)
                          ;; Skipping to the end of the specified region
@@ -1220,563 +1306,269 @@
 	 (eval-defun (not edebug-all-defs)))
 	(t
 	 (if (null eval-expression-debug-on-error)
-	     (eval-defun-2)
-	   (let ((old-value (make-symbol "t")) new-value value)
-	     (let ((debug-on-error old-value))
-	       (setq value (eval-defun-2))
+	     (elisp--eval-defun)
+	   (let (new-value value)
+	     (let ((debug-on-error elisp--eval-last-sexp-fake-value))
+	       (setq value (elisp--eval-defun))
 	       (setq new-value debug-on-error))
-	     (unless (eq old-value new-value)
+	     (unless (eq elisp--eval-last-sexp-fake-value new-value)
 	       (setq debug-on-error new-value))
 	     value)))))
 
-;; May still be used by some external Lisp-mode variant.
-(define-obsolete-function-alias 'lisp-comment-indent
-    'comment-indent-default "22.1")
-(define-obsolete-function-alias 'lisp-mode-auto-fill 'do-auto-fill "23.1")
+;;; ElDoc Support

bug#19362: 25.0.50; Fix `pp.el' in line with new `elisp-mode.el'

[Drew Adams]

> Well, we could have all saved some time if you had taken your own advice
> to diff the code; not pp.el vs elisp-mode.el, but emacs-24.5's
> lisp-mode.el vs the new elisp-mode.el.  You can see in the attached diff
> (exerpted to leave only the relevant functions) that the only changes
> are renaming of functions, and some minor refactoring in
> elisp--preceding-sexp (it now handles ‘foo’ as well as `foo').

I never suggested diffing pp.el against either lisp-mode.el or
elisp-mode.el.  And actually I did compare lisp-mode.el with
elisp-mode.el and different versions of lisp-mode.el.  I said
this, which I maintain:

> Yes, well, it's quite difficult.  Some of the code from lisp-mode.el
> was moved to elisp-mode.el.  And then it was modified, in some cases
> radically.  Dunno why, except that some of the changes seem to have
> involved integrating eldoc.  But most of the changes presumably do not
> concern this bug, which is only about the part of lisp-mode.el that
> had functions that correspond to pp.el functions.)

It should be clear that wrt what I was following (and comparing)
of the evolution, what I saw was not just renaming of functions
and minor refactoring.  But of course I was also trying to see
how the changes compared to the pp.el code - what corresponding
changes might be in order for pp.el.  That was, uh, the point of
the report.  Whatever.  Thanks for taking a look anyway.  Sorry
that you apparently wasted your time.