bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Visuwesh]

The notion of "last sexp" is different for eval-last-sexp and
pp-eval-last-sexp as seen by the following,

    ,(list 1 2 3)

eval-last-sexp echoes (1 2 3) in the echo area but pp-eval-last-sexp
signals an error.  I am not sure which one is more correct, but
eval-last-sexp behaviour is more convenient (and backward-sexp when
after the closing parenthesis places the point after , too...).

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw scroll bars)
Repository revision: ca3858563c7ba8ee3caa82fbd2b7c386ea60c0d3
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: NixOS 21.11 (Porcupine)

Configured using:
 'configure
 --prefix=/nix/store/iqqk7iqfwmfc6r78xg2knyq7hww2mhs4-emacs-git-20220225.0
 --disable-build-details --with-modules --with-x-toolkit=lucid
 --with-xft --with-cairo --with-native-compilation'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM LUCID ZLIB

Important settings:
  value of $EMACSLOADPATH: 
  value of $EMACSNATIVELOADPATH: /nix/store/5gh4w50dhchhcyjm6ysh17h7y4i5vasf-emacs-packages-deps/share/emacs/native-lisp::
  value of $LC_MONETARY: ta_IN.UTF-8
  value of $LC_NUMERIC: ta_IN.UTF-8
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  semantic-minor-modes-format: ((:eval (if (or semantic-highlight-edits-mode semantic-show-unmatched-syntax-mode)  S)))
  recentf-mode: t
  shell-dirtrack-mode: t
  paredit-mode: t
  eros-mode: t
  flymake-mode: t
  hl-todo-mode: t
  pdf-occur-global-minor-mode: t
  minibuffer-depth-indicate-mode: t
  repeat-mode: t
  display-time-mode: t
  display-battery-mode: t
  straight-use-package-mode: t
  straight-package-neutering-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tab-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  undelete-frame-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/viz/.nix-profile/share/emacs/site-lisp/site-start hides /nix/store/5gh4w50dhchhcyjm6ysh17h7y4i5vasf-emacs-packages-deps/share/emacs/site-lisp/site-start
/home/viz/lib/emacs/straight/build/map/map hides /nix/store/iqqk7iqfwmfc6r78xg2knyq7hww2mhs4-emacs-git-20220225.0/share/emacs/29.0.50/lisp/emacs-lisp/map
/home/viz/lib/emacs/straight/build/let-alist/let-alist hides /nix/store/iqqk7iqfwmfc6r78xg2knyq7hww2mhs4-emacs-git-20220225.0/share/emacs/29.0.50/lisp/emacs-lisp/let-alist

Features:
(shadow emacsbug sendmail ecomplete debug tamil-phonetic gnus-dired
flow-fill mm-archive sort gnus-cite mail-extr gnus-async gnus-bcklg qp
gnus-ml nndraft nnmh nnfolder nnmaildir nnagent nnml nnnil gnus-agent
gnus-srvr gnus-score score-mode nnvirtual gnus-msg nntp gnus-cache
info-look org-archive expand-region text-mode-expansions
cc-mode-expansions the-org-mode-expansions er-basic-expansions
expand-region-core expand-region-custom edebug backtrace cal-iso view
org-clock org-plot ob-ditaa ob-plantuml org-crypt org-habit descr-text
man wdired ement-room-list ts s dash ement ement-notify notifications
ement-room ewoc ement-api ement-structs ement-macros plz dns ind-util
writegood-mode cal-islam holidays hol-loaddefs mule-util cal-move tabify
org-datetree org-capture doct org-agenda org-colview org-modern
org-modern-autoloads autoload hippie-exp smerge-mode diff log-edit
pcvs-util add-log vc image-file image-converter timezone comp comp-cstr
shortdoc xref elec-pair flyspell ispell org-pdftools org-noter
org-refile org-indent org-element avl-tree generator executable
time-stamp pulse dired-aux shr-color color textsec uni-scripts
idna-mapping ucs-normalize uni-confusable textsec-check gnutls url-http
url-gw url-cache url-auth goto-addr icomplete pdf-sync pdf-annot
facemenu pdf-outline pdf-links ob-C cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs ob-shell ob-racket
async ob-async tempo ol-eww eww xdg url-queue mm-url ol-rmail ol-mhe
ol-irc ol-info ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime
smime dig gnus-sum shr pixel-fill kinsoku url-file url-dired svg dom
gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud nnimap nnmail
mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range message
yank-media rfc822 mml mml-sec epa epg rfc6068 epg-config mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums
mailabbrev gmm-utils mailheader gnus-win gnus nnheader gnus-util
mail-utils range mm-util mail-prsvr ol-docview doc-view ol-bibtex
ol-bbdb ol-w3m ol-doi org-link-doi org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete
org-list org-faces org-entities org-version ob-emacs-lisp ob-core
ob-eval org-table oc-basic bibtex ol org-keys oc org-compat org-macs
org-loaddefs pdf-history network-stream puny nsm rmc dictionary
dictionary-connection misearch multi-isearch reveal noutline outline
cl-print help-fns radix-tree recentf tree-widget vc-git diff-mode
vc-dispatcher tramp-cmds rfc2104 tramp-cache tramp-sh tramp
tramp-loaddefs trampver tramp-integration cus-start files-x tramp-compat
parse-time iso8601 ls-lisp shell-command+ cursor-sensor face-remap shell
pcomplete server paredit edmacro kmacro eros time-date checkdoc lisp-mnt
flymake-proc flymake project warnings thingatpt hl-todo wordel-autoloads
sokoban-autoloads ement-autoloads ts-autoloads map-autoloads
plz-autoloads nov-autoloads esxml-autoloads kv-autoloads
transmission-autoloads lua-mode-autoloads nix-mode-autoloads
magit-section-autoloads dash-autoloads racket-mode-autoloads
eros-autoloads flymake-shellcheck-autoloads writegood-mode-autoloads avy
avy-autoloads siege-mode-autoloads paredit-autoloads puni-autoloads
expand-region-autoloads filladapt-autoloads compose quail
scroll-other-window org-pdftools-autoloads org-noter-autoloads
math-delimiters-autoloads doct-autoloads ob-async-autoloads
async-autoloads emacs-ob-racket-autoloads valign-autoloads
org-starless-autoloads cdlatex-autoloads auctex-autoloads tex-site
easy-mmode pdf-occur ibuf-ext ibuffer ibuffer-loaddefs tablist advice
tablist-filter semantic/wisent/comp semantic/wisent
semantic/wisent/wisent semantic/util-modes semantic/util semantic
semantic/tag semantic/lex semantic/fw mode-local find-func cedet
pdf-isearch let-alist pdf-misc imenu pdf-tools package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util mailcap url-handlers url-parse auth-source eieio
eieio-core eieio-loaddefs json map url-vars compile comint ansi-color
ring cus-edit wid-edit pdf-view password-cache jka-compr pdf-cache
pdf-info tq pdf-util pdf-macs image-mode dired-x dired dired-loaddefs
exif pdf-tools-autoloads let-alist-autoloads tablist-autoloads derived
mb-depth cus-load repeat visual-fill-autoloads olivetti-autoloads
hl-todo-autoloads time format-spec battery dbus filenotify xml
disp-table lacarte-autoloads shell-command-plus-autoloads icalendar
diary-lib diary-loaddefs cal-menu calendar cal-loaddefs rx filecache
flymake-grammarly-autoloads grammarly-autoloads websocket-autoloads
finder-inf request-autoloads s-autoloads chemtable-autoloads
molar-mass-autoloads saveplace-pdf-view saveplace bookmark
text-property-search pp saveplace-pdf-view-autoloads pcase
straight-autoloads info cl-seq cl-extra help-mode straight cl-macs
cl-loaddefs cl-lib vz-nh-theme seq gv subr-x byte-opt bytecomp
byte-compile cconv iso-transl tooltip eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
x multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 1759947 339294)
 (symbols 48 59226 52)
 (strings 32 341856 24192)
 (string-bytes 1 63484760)
 (vectors 16 139758)
 (vector-slots 8 3389303 393319)
 (floats 8 30915 3058)
 (intervals 56 131235 5086)
 (buffers 992 107))

bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Lars Ingebrigtsen]

Visuwesh <visuweshm@gmail.com> writes:

> The notion of "last sexp" is different for eval-last-sexp and
> pp-eval-last-sexp as seen by the following,
>
>     ,(list 1 2 3)
>
> eval-last-sexp echoes (1 2 3) in the echo area but pp-eval-last-sexp
> signals an error.  I am not sure which one is more correct, but
> eval-last-sexp behaviour is more convenient (and backward-sexp when
> after the closing parenthesis places the point after , too...).

The pp function uses (forward-sexp -1) to determine where the preceding
expression begins, which seems natural.  (And it skips to before the
comma.)

The non-pp function uses (elisp--preceding-sexp), which basically does
the same, but then explicitly skips past the comma.

I think the non-pp function has the most convenient behaviour, so I'd be
in favour of changing it to just use (elisp--preceding-sexp).  Any opinions?

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

bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Drew Adams]

> I think the non-pp function has the most convenient behaviour, so I'd
> be in favour of changing it to just use (elisp--preceding-sexp).  Any
> opinions?

Whether it's better ("more convenient") or not,
I don't know.  But it's a backward-incompatible
change.  That might be one thing to consider -
what users expect will break/change.

bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Visuwesh]

[புதன், மார்ச் 23 2022] Drew Adams wrote:

>> I think the non-pp function has the most convenient behaviour, so I'd
>> be in favour of changing it to just use (elisp--preceding-sexp).  Any
>> opinions?
>
> Whether it's better ("more convenient") or not,
> I don't know.  But it's a backward-incompatible
> change.  That might be one thing to consider -
> what users expect will break/change.

Sure, but the current (inconsistent) behaviour is surprising.  I would
be personally okay with such a backwards-incompatible change, but I am
biased since I filed the bug report.

bug#54537: Re: bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Drew Adams]

> > Whether it's better ("more convenient") or not,
> > I don't know.  But it's a backward-incompatible
> > change.  That might be one thing to consider -
> > what users expect will break/change.
> 
> Sure, but the current (inconsistent) behaviour is surprising.

Is it?  The `pp-*' commands have different uses.

FWIW, I even add separate variables for PP, for
controlling print level and length.  Force-using
the same values for the `pp-*' commands doesn't
make sense to me.

Similarly, I'm not shocked by the difference this
bug report wants to get rid of.  I don't necessarily
object to the change.  I'm just not convinced that
such consistency is important.

What matters most wrt consistency is _local_
consistency - being consistent within a given
context/scope/set of rules.  PP is a different
world (context).

(That's not an argument why PP's last-sexp should
be different.  It's an argument that just a "more
convenient" argument that it should be the same
isn't a strong one.)

> I would be personally okay with such a
> backwards-incompatible change, but I am
> biased since I filed the bug report.

It's a reasonable proposal to examine.  And it
might be the right thing to do.

My contribution is just to point out that PP eval
is not non-PP eval, and "more convenient" isn't a
strong argument when the increment of "more" isn't
large.  And when introducing backward-incompatible
behavior maybe other, stronger arguments would help.

Maybe bring this up on emacs-devel, to see if more
and better arguments for and against the change
are available?  (Not a requirement, of course.)

bug#54537: Re: bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Michael Heerdegen]

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

> > Sure, but the current (inconsistent) behaviour is surprising.
>
> Is it?  The `pp-*' commands have different uses.
>
> FWIW, I even add separate variables for PP, for
> controlling print level and length.  Force-using
> the same values for the `pp-*' commands doesn't
> make sense to me.
>
> Similarly, I'm not shocked by the difference this
> bug report wants to get rid of.  I don't necessarily
> object to the change.  I'm just not convinced that
> such consistency is important.

Could as well be that it's just only different because nobody cared
enough about pp all the time.  I would not be shocked if the different
behavior is only an accident.

Michael.

bug#54537: Re: bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Drew Adams]

> Could as well be that it's just only different because nobody cared
> enough about pp all the time.  I would not be shocked if the different
> behavior is only an accident.

Could be.  Dunno.

Still calls out for a better argument than a vague
opinion that such a change adding incompatibility
would be "more convenient".  That's all.

The commands have different uses.  Their behavior
has long been different in this regard.  What's a
great argument for making this change?  That's the
question.

As I said:

  It's a reasonable proposal to examine.  And it
  might be the right thing to do.

  PP eval is not non-PP eval, and "more convenient"
  isn't a strong argument when the increment of
  "more" isn't large.

  When introducing backward-incompatible behavior
  maybe other, stronger arguments would help.

bug#54537: Re: bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Michael Heerdegen]

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

> Still calls out for a better argument than a vague
> opinion that such a change adding incompatibility
> would be "more convenient".  That's all.

Ok - let's become concrete now.

About what kinds of use cases are we a talking about?  Use cases like
this:

#+begin_src emacs-lisp
`(,(+ 2 3)
  ,(+ 4 5))
#+end_src

The non-pp behavior is useful (you are able to eval the inserted
subexpressions).  The pp behavior is not useful (error).

This behaves identically in both versions:

#+begin_src emacs-lisp
',(+ 4 5)
#+end_src

(you get the expected `,(+ 4 5)`.)

So we only talk about plain naked `unquote` expressions.

Do you see any concrete advantages of the pp-version behavior?  Or some
concrete hints that the pp version must be like this for more
consistency in the pp package?

OTOH, a concrete problem I see is that people avoid pp due to such things.
As far as I recall the history of the pp package, I don't expect much
logic behind the behavior.  Maybe it really just...sucks?

Michael.

bug#54537: Re: bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Drew Adams]

> `(,(+ 2 3)
>   ,(+ 4 5))
> 
> The non-pp behavior is useful (you are able to eval the inserted
> subexpressions).  The pp behavior is not useful (error).

OK, I'm convinced.  I didn't (and I don't) see a good
reason for the `pp-*' behavior is that regard.  Just
wanted people to take a look.

What's needed is a fixed version of `pp-last-sexp'.
It's not about `pp-eval-last-sexp'.

pp-last-sexp' has several differences from
`elisp--preceding-sexp'.  Should some of those
differences be kept?  Dunno.  E.g.,  it uses `read',
and it ignores leading comments.

Regardless of what changes are made to `pp-last-sexp',
I think those changes should allow the functions that
call `pp-last-sexp' to remain unchanged; IOW, fix
`pp-last-sexp', but don't change how it's used/called.

E.g., `pp-eval-last-sexp' should remain with this code:

(defun pp-eval-last-sexp (arg)
  "..."
  (interactive "P")
  (if arg
      (insert (pp-to-string (eval (pp-last-sexp)
                                  lexical-binding)))
    (pp-eval-expression (pp-last-sexp))))

Similarly `pp-macroexpand-last-sexp'.  There's no doubt
other code out there (I have some) that makes use of
`pp-last-sexp'.  Let's please have just a simple fix of
`pp-last-sexp'.

And besides allowing no changes in calling, preferably
no other behavior would be changed than to fix this bug.

bug#54537: Re: bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Visuwesh]

[புதன், மார்ச் 23 2022] Drew Adams wrote:

>> > Whether it's better ("more convenient") or not,
>> > I don't know.  But it's a backward-incompatible
>> > change.  That might be one thing to consider -
>> > what users expect will break/change.
>> 
>> Sure, but the current (inconsistent) behaviour is surprising.
>
> Is it?  The `pp-*' commands have different uses.
>

They do, and I don't object to that.  But when I see pp-eval-last-sexp,
I think it would do the same thing as eval-last-sexp but pretty prints
the result in a separate buffer instead.  In this regard, which I think
is a reasonable interpretation, pp-eval-last-sexp is not inconsistent
and hence behaves in an unexpected manner.

> FWIW, I even add separate variables for PP, for
> controlling print level and length.  Force-using
> the same values for the `pp-*' commands doesn't
> make sense to me.
>
> Similarly, I'm not shocked by the difference this
> bug report wants to get rid of.  I don't necessarily
> object to the change.  I'm just not convinced that
> such consistency is important.
>

I would agree with you if this change removed an ability to do a certain
thing in the name of consistency but here, it merely changes the
behaviour to something more convenient.  I see no point in bikeshedding
further however, so I will stop here.  The final decision will be up to
the maintainers.

> What matters most wrt consistency is _local_
> consistency - being consistent within a given
> context/scope/set of rules.  PP is a different
> world (context).
>
> (That's not an argument why PP's last-sexp should
> be different.  It's an argument that just a "more
> convenient" argument that it should be the same
> isn't a strong one.)
>
>> I would be personally okay with such a
>> backwards-incompatible change, but I am
>> biased since I filed the bug report.
>
> It's a reasonable proposal to examine.  And it
> might be the right thing to do.
>
> My contribution is just to point out that PP eval
> is not non-PP eval, and "more convenient" isn't a
> strong argument when the increment of "more" isn't
> large.  And when introducing backward-incompatible
> behavior maybe other, stronger arguments would help.
>
> Maybe bring this up on emacs-devel, to see if more
> and better arguments for and against the change
> are available?  (Not a requirement, of course.)

bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Lars Ingebrigtsen]

Visuwesh <visuweshm@gmail.com> writes:

> The notion of "last sexp" is different for eval-last-sexp and
> pp-eval-last-sexp as seen by the following,
>
>     ,(list 1 2 3)
>
> eval-last-sexp echoes (1 2 3) in the echo area but pp-eval-last-sexp
> signals an error.  I am not sure which one is more correct, but
> eval-last-sexp behaviour is more convenient (and backward-sexp when
> after the closing parenthesis places the point after , too...).

I've now fixed this in Emacs 29.

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

bug#54537: 29.0.50; Last sexp notion is different for eval-last-sexp and pp-eval-last-sexp

[Visuwesh]

[வெள்ளி, மார்ச் 25, 2022] Lars Ingebrigtsen wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> The notion of "last sexp" is different for eval-last-sexp and
>> pp-eval-last-sexp as seen by the following,
>>
>>     ,(list 1 2 3)
>>
>> eval-last-sexp echoes (1 2 3) in the echo area but pp-eval-last-sexp
>> signals an error.  I am not sure which one is more correct, but
>> eval-last-sexp behaviour is more convenient (and backward-sexp when
>> after the closing parenthesis places the point after , too...).
>
> I've now fixed this in Emacs 29.

Great, thanks!