unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
@ 2020-09-11 10:10 Philipp Stephani
  2020-09-11 10:26 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Philipp Stephani @ 2020-09-11 10:10 UTC (permalink / raw)
  To: 43329


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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
  2020-09-11 10:10 bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form Philipp Stephani
@ 2020-09-11 10:26 ` Lars Ingebrigtsen
  2020-09-11 16:35   ` Noam Postavsky
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-11 10:26 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 43329

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
  2020-09-11 10:26 ` Lars Ingebrigtsen
@ 2020-09-11 16:35   ` Noam Postavsky
  2020-09-12 11:57     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Noam Postavsky @ 2020-09-11 16:35 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Philipp Stephani, 43329

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)





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
  2020-09-11 16:35   ` Noam Postavsky
@ 2020-09-12 11:57     ` Lars Ingebrigtsen
  2020-09-12 15:48       ` Stefan Monnier
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-12 11:57 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Philipp Stephani, Stefan Monnier, 43329

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





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
  2020-09-12 11:57     ` Lars Ingebrigtsen
@ 2020-09-12 15:48       ` Stefan Monnier
  2020-09-13 12:44         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2020-09-12 15:48 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Philipp Stephani, Noam Postavsky, 43329

> 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






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
  2020-09-12 15:48       ` Stefan Monnier
@ 2020-09-13 12:44         ` Lars Ingebrigtsen
  2020-09-13 15:24           ` Stefan Monnier
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-13 12:44 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Philipp Stephani, Noam Postavsky, 43329

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
  2020-09-13 12:44         ` Lars Ingebrigtsen
@ 2020-09-13 15:24           ` Stefan Monnier
  2020-09-13 17:20             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2020-09-13 15:24 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Philipp Stephani, Noam Postavsky, 43329

> 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






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
  2020-09-13 15:24           ` Stefan Monnier
@ 2020-09-13 17:20             ` Lars Ingebrigtsen
  2021-10-13 19:23               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-13 17:20 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Philipp Stephani, Noam Postavsky, 43329, João Távora

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form
  2020-09-13 17:20             ` Lars Ingebrigtsen
@ 2021-10-13 19:23               ` Lars Ingebrigtsen
  2021-10-18  8:00                 ` bug#43329: bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2021-10-13 19:23 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Philipp Stephani, Noam Postavsky, 43329, João Távora

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#43329: bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.."
  2021-10-13 19:23               ` Lars Ingebrigtsen
@ 2021-10-18  8:00                 ` Lars Ingebrigtsen
  0 siblings, 0 replies; 10+ messages in thread
From: Lars Ingebrigtsen @ 2021-10-18  8:00 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Philipp Stephani, 33301, Noam Postavsky, 43329,
	João Távora

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-10-18  8:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-11 10:10 bug#43329: 28.0.50; Incorrect indentation for variables starting with `def' in let form Philipp Stephani
2020-09-11 10:26 ` Lars Ingebrigtsen
2020-09-11 16:35   ` Noam Postavsky
2020-09-12 11:57     ` Lars Ingebrigtsen
2020-09-12 15:48       ` Stefan Monnier
2020-09-13 12:44         ` Lars Ingebrigtsen
2020-09-13 15:24           ` Stefan Monnier
2020-09-13 17:20             ` Lars Ingebrigtsen
2021-10-13 19:23               ` Lars Ingebrigtsen
2021-10-18  8:00                 ` bug#43329: bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).