bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Philipp Stephani]

In the scratch buffer, enter

(let ((defa
       foo)))

and reindent the second line.  Emacs will insert an additional space
before `foo' so that the `f' is now below the `e'.  This seems to happen
only for variables starting with `def' (but not `def' itself).


In GNU Emacs 28.0.50 (build 90, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
 of 2020-09-11
Repository revision: e2391d486e8a97e383db2337fad6a93c2c11656a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Debian GNU/Linux rodete

Configured using:
 'configure --enable-gcc-warnings=warn-only
 --enable-gtk-deprecation-warnings --without-pop --with-mailutils
 --enable-checking=all --enable-check-lisp-object-type --with-modules
 'CFLAGS=-O1 -ggdb3 -fno-omit-frame-pointer -fsanitize=address
 -fsanitize=undefined -fsanitize=pointer-compare
 -fsanitize=pointer-subtract''

Configured features:
XPM JPEG TIFF GIF PNG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY INOTIFY
LIBSELINUX GNUTLS FREETYPE HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS GTK3 X11
XDBE XIM MODULES THREADS LIBSYSTEMD JSON PDUMPER

Important settings:
  value of $LANG: en_US.utf8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc dired dired-loaddefs rfc822
mml easymenu mml-sec epa epg epg-config gnus-util rmail rmail-loaddefs
text-property-search time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils phst skeleton derived edmacro
kmacro pcase ffap thingatpt url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf url-util url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json map url-vars mailcap subr-x rx gnutls puny seq
byte-opt gv bytecomp byte-compile cconv dbus xml compile comint
ansi-color ring cl-loaddefs cl-lib tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type 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 elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu 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 charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 68841 6345)
 (symbols 48 8619 1)
 (strings 32 23832 1278)
 (string-bytes 1 770267)
 (vectors 16 13727)
 (vector-slots 8 188001 5269)
 (floats 8 26 31)
 (intervals 56 218 0)
 (buffers 992 11))

-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich.  Falls Sie diese fälschlicherweise erhalten haben
sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie
alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail
an die falsche Person gesendet wurde.

This e-mail is confidential.  If you received this communication by mistake,
please don’t forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Lars Ingebrigtsen]

Philipp Stephani <p.stephani2@gmail.com> writes:

> In the scratch buffer, enter
>
> (let ((defa
>        foo)))
>
> and reindent the second line.  Emacs will insert an additional space
> before `foo' so that the `f' is now below the `e'.  This seems to happen
> only for variables starting with `def' (but not `def' itself).

Yes, the Emacs Lisp indentation machinery assumes that everything that
starts with "def" is a macro that defines something.  That's pretty
quirky, but it's been that way since forever, so it's probably too late
to do something about that now.

It should, however, be possible to do something about this in a `let'
form, you'd think.  But I've tried to find the place where this happens
(I was looking at it just the other day!), but my grep skills fail me.

Anybody remember which function this happens in?

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

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Noam Postavsky]

forcemerge 33301 43329
quit

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yes, the Emacs Lisp indentation machinery assumes that everything that
> starts with "def" is a macro that defines something.
[...]
> Anybody remember which function this happens in?

lisp-indent-function (see https://debbugs.gnu.org/33301#15)

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Lars Ingebrigtsen]

Noam Postavsky <npostavs@gmail.com> writes:

> lisp-indent-function (see https://debbugs.gnu.org/33301#15)

Ah, yes, there it is:

(defun lisp-indent-function (indent-point state)
[...]
	(cond ((or (eq method 'defun)
		   (and (null method)
			(> (length function) 3)
			(string-match "\\`def" function)))
	       (lisp-indent-defform state indent-point))

I think the general idea here is to indent everything that starts with
"(def" as a macro, but in

(let ((defa
       foo)))

that's obviously not the case here.

Would something as simple as the following (which fixes this use case)
be the correct fix?  It only indents these forms as macros if they are
top-level...

("make check" signals no errors with the change in place.)

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index 352210f859..a324846336 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -1141,6 +1141,7 @@ lisp-indent-function
 	(cond ((or (eq method 'defun)
 		   (and (null method)
 			(> (length function) 3)
+                        (= (syntax-ppss-depth state) 1)
 			(string-match "\\`def" function)))
 	       (lisp-indent-defform state indent-point))
 	      ((integerp method)


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

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Stefan Monnier]

> Would something as simple as the following (which fixes this use case)
> be the correct fix?  It only indents these forms as macros if they are
> top-level...

It obviously has the downside of not indenting properly those
definitions which are not at the very-top level.  E.g. definitions
inside an `eval-when-compile` or other.


        Stefan

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Would something as simple as the following (which fixes this use case)
>> be the correct fix?  It only indents these forms as macros if they are
>> top-level...
>
> It obviously has the downside of not indenting properly those
> definitions which are not at the very-top level.  E.g. definitions
> inside an `eval-when-compile` or other.

That's true.  I guess the (def* calls can happen at any level, really.

But the (def* thing is a heuristic, and we could say that we only use
that heuristic at the top level?

We should then mark all the (def* things we have in-tree with explicit
indentation marking (which I think we should do anyway, really).

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

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Stefan Monnier]

> But the (def* thing is a heuristic, and we could say that we only use
> that heuristic at the top level?

We can replace one heuristic with another, yes.
That exchanges misindentationss for other misindentations.

> We should then mark all the (def* things we have in-tree with explicit
> indentation marking (which I think we should do anyway, really).

Indeed (and I think we've been getting there bit by bit.  Not sure how
far we stand from that nowadays).  Of course, that won't help with

    (let ((defvar
           3))
      (+ defvar 5))

and other oddball cases ;-)


        Stefan

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Indeed (and I think we've been getting there bit by bit.  Not sure how
> far we stand from that nowadays).

João suggested over on bug#33301 to only make the heuristic happen for
macros, and I was sceptical, but:

> Indeed I don't know how to proceed.  There seem to be around 34. 
> Add indent specs for these 34 symbols? I suspect that some have exclusively
> one-line uses that don't need them.

So 34 in-tree functions that start with "def", but an unknown number of
macros.  That's not a lot, and I think we should add indent specs to all
of them, including the macros.

> Of course, that won't help with
>
>     (let ((defvar
>            3))
>       (+ defvar 5))
>
> and other oddball cases ;-)

Heh, no.

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

bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So 34 in-tree functions that start with "def", but an unknown number of
> macros.  That's not a lot, and I think we should add indent specs to all
> of them, including the macros.

I've now gone through all the functions that have names that start with
"def" and marked the ones that should have `defun' indent explicitly.
I'll do the macros next, and then we can remove the heuristic.

This will change how out-of-tree code indents, though.  This can be
fixed by out-of-tree packages doing markup, but is there some way we can
make the transition kinder?  (I don't think so, though.)

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

bug#43329: bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.."

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> This will change how out-of-tree code indents, though.  This can be
> fixed by out-of-tree packages doing markup, but is there some way we can
> make the transition kinder?  (I don't think so, though.)

I've now pushed this change to Emacs 29.

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