GNU bug report logs - #33301
27.0.50; broken elisp indentation for non-definition symbols starting with "def.."

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 33301 in the body.
You can then email your comments to 33301 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Wed, 07 Nov 2018 13:21:39 +0000

Hi maintainers,

This doesn't seem the correct indentation for the following elisp forms:

(let ((bla
       (ok))
      (defan
	(strange))))

(cond (bla
       ok)
      (defan
        strange))

...but that's the way Emacs -Q does it.  I'd be suprised if this weren't
a duplicate, but I thought I'd report it just in case.

Thanks,
João

Message #8 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 33301 <at> debbugs.gnu.org
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Wed, 07 Nov 2018 19:05:37 -0500

found 33301 24.3
tags 33301 + confirmed
quit

João Távora <joaotavora <at> gmail.com> writes:

> This doesn't seem the correct indentation for the following elisp forms:
>
> (let ((bla
>        (ok))
>       (defan
>         (strange))))
>
> (cond (bla
>        ok)
>       (defan
>         strange))
>
> ...but that's the way Emacs -Q does it.  I'd be suprised if this weren't
> a duplicate, but I thought I'd report it just in case.

I can't find any duplicate, though it's certainly not new.  Bug#9622 and
Bug#23108 are sort of related.

Message #15 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 33301 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Thu, 08 Nov 2018 01:35:41 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

> > (cond (bla
> >        ok)
> >       (defan
> >         strange))

That's explicitly done in 'lisp-indent-function':

(cond ((or (eq method 'defun)
		   (and (null method)
			(> (length function) 3)
			(string-match "\\`def" function))) ;; <==
	       (lisp-indent-defform state indent-point))
	      ((integerp method)
	       (lisp-indent-specform method state
				     indent-point normal-indent))
	      (method
		(funcall method indent-point state)))


Michael.

Message #18 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 33301 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Thu, 08 Nov 2018 09:52:51 +0000

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Noam Postavsky <npostavs <at> gmail.com> writes:
>
>> > (cond (bla
>> >        ok)
>> >       (defan
>> >         strange))
>
> That's explicitly done in 'lisp-indent-function':
> 		   (and (null method)
> 			(> (length function) 3)
> 			(string-match "\\`def" function))) ;; <==

Ah, that's unfortunate.  Still, coundn't we improve the heuristic by
asking if the "function" has a macro definition?  Isn't that closer to
the intended behaviour?

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index afb7cbd1dd..e7373ece85 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -1104,7 +1104,8 @@ lisp-indent-function
 	(cond ((or (eq method 'defun)
 		   (and (null method)
 			(> (length function) 3)
-			(string-match "\\`def" function)))
+			(string-match "\\`def" function)
+			(macrop (intern function))))
 	       (lisp-indent-defform state indent-point))
 	      ((integerp method)
 	       (lisp-indent-specform method state

João

Message #21 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: João Távora <joaotavora <at> gmail.com>
Cc: 33301 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Fri, 09 Nov 2018 01:13:20 +0100

João Távora <joaotavora <at> gmail.com> writes:

> Ah, that's unfortunate.  Still, coundn't we improve the heuristic by
> asking if the "function" has a macro definition?  Isn't that closer to
> the intended behaviour?
>
> diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
> index afb7cbd1dd..e7373ece85 100644
> --- a/lisp/emacs-lisp/lisp-mode.el
> +++ b/lisp/emacs-lisp/lisp-mode.el
> @@ -1104,7 +1104,8 @@ lisp-indent-function
>  	(cond ((or (eq method 'defun)
>  		   (and (null method)
>  			(> (length function) 3)
> -			(string-match "\\`def" function)))
> +			(string-match "\\`def" function)
> +			(macrop (intern function))))
>  	       (lisp-indent-defform state indent-point))
>  	      ((integerp method)
>  	       (lisp-indent-specform method state

If that macro is defined, shouldn't it already be indented correctly
without heuristics?

Michael.

Message #24 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 33301 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Fri, 09 Nov 2018 00:41:53 +0000

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>
>> Ah, that's unfortunate.  Still, coundn't we improve the heuristic by
>> asking if the "function" has a macro definition?  Isn't that closer to
>> the intended behaviour?
>>
>> diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
>> index afb7cbd1dd..e7373ece85 100644
>> --- a/lisp/emacs-lisp/lisp-mode.el
>> +++ b/lisp/emacs-lisp/lisp-mode.el
>> @@ -1104,7 +1104,8 @@ lisp-indent-function
>>  	(cond ((or (eq method 'defun)
>>  		   (and (null method)
>>  			(> (length function) 3)
>> -			(string-match "\\`def" function)))
>> +			(string-match "\\`def" function)
>> +			(macrop (intern function))))
>>  	       (lisp-indent-defform state indent-point))
>>  	      ((integerp method)
>>  	       (lisp-indent-specform method state
>
> If that macro is defined, shouldn't it already be indented correctly
> without heuristics?

I don't think so, not without an explicit indent declaration spec in the
macro definition.

This may explain the string-match hack in the first place. I don't know
the exact motivation of the hack, but it's been there since the initial
2001 revision of the file.  Possibly before declare/indent existed?

If you're suggesting removing it entirely, I don't oppose it.  There's
the downside that indentation relying on it would start to fail, but
diffs normally spot that and this would encourage users to add proper
indent (and edebug) specs to their macros.

Otherwise, I think my macrop tweak does a slightly better job at
avoiding false positives.

João

Message #27 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: João Távora <joaotavora <at> gmail.com>
Cc: 33301 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Fri, 09 Nov 2018 02:45:01 +0100

João Távora <joaotavora <at> gmail.com> writes:

> This may explain the string-match hack in the first place. I don't know
> the exact motivation of the hack, but it's been there since the initial
> 2001 revision of the file.  Possibly before declare/indent existed?

But wait, this is in lisp-mode.el which I remember is used not only for
Elisp but also for other Lisps, right?  So your patch could make things
worse for editing Common Lisp, for example.

For Elisp the heuristic doesn't make much sense, though, if the edited
file is not loaded, it also prevents false negatives for macro uses of
macros defined in that file.


Michael.

Message #30 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 33301 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Fri, 09 Nov 2018 09:04:04 +0000

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>
>> This may explain the string-match hack in the first place. I don't know
>> the exact motivation of the hack, but it's been there since the initial
>> 2001 revision of the file.  Possibly before declare/indent existed?
>
> But wait, this is in lisp-mode.el which I remember is used not only for
> Elisp but also for other Lisps, right?

Well it's lisp/emacs-lisp/... ;-)

> So your patch could make things worse for editing Common Lisp, for
> example.

OK, just add (derived-mode-p 'emacs-lisp-mode), as is done elsewhere in
that file.

Or I would suggest (setq-local lisp-indent-function
'common-lisp-indent-function) in you hypothetical fancy-lisp-mode hook
and has much better heuristics that don't cause the bug I've described.

(But, as someone who writes CL for a living, if you're indenting CL with
these heuristics, you've already lost.  You should use SLY/SLIME which
looks at the macroexpansion to understand what you're trying to indent.)

> For Elisp the heuristic doesn't make much sense, though, if the edited
> file is not loaded, it also prevents false negatives for macro uses of
> macros defined in that file.

I don't fully understand the "it also" part, but here's my take on this:
If you're not loading the code, all things being equal, it's better to
incorrectly re-indent existing "def"-macros (not defmacro) than to
incorrectly indent new arbitrary "def"-forms anywhere in the AST.
That's because it's a bad idea to re-indent code anyway, but indent new
code happens all the time.

Also, it's not a very good idea to indent without some form of
evaluation anyway.  Because of the indentation declaration, that ship
has sailed long ago (and bon voyage).

João

Message #33 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: João Távora <joaotavora <at> gmail.com>
Cc: 33301 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Fri, 09 Nov 2018 10:51:55 +0100

João Távora <joaotavora <at> gmail.com> writes:

> OK, just add (derived-mode-p 'emacs-lisp-mode), as is done elsewhere
> in that file.

I think we could do that, but I don't feel qualified to decide.  Maybe
Noam can help.

A quick search shows that the "\\`def" trick is also performed
elsewhere, btw: `common-lisp-indent-function-1' does it in one place,
and also `scheme-indent-function'.

As far as the example in your bug report is concerned, I think it would
also be an improvement if elisp-mode wouldn't try be clever in such a
way when indenting branches in a cond or variable associations in a let.


Michael.

Message #36 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 33301 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Fri, 09 Nov 2018 07:28:53 -0500

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>
>> OK, just add (derived-mode-p 'emacs-lisp-mode), as is done elsewhere
>> in that file.
>
> I think we could do that, but I don't feel qualified to decide.  Maybe
> Noam can help.

I think it would be acceptable.

> As far as the example in your bug report is concerned, I think it would
> also be an improvement if elisp-mode wouldn't try be clever in such a
> way when indenting branches in a cond or variable associations in a let.

The problem is that we currently don't have any way of specifying
indentation for subforms of macro arguments, which is also the core
problem of the other bugs I mentioned about indentation of cl-flet and
friends.

Message #39 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Fri, 09 Nov 2018 19:39:16 +0000

Noam Postavsky <npostavs <at> gmail.com> writes:

> Michael Heerdegen <michael_heerdegen <at> web.de> writes:
>
>> João Távora <joaotavora <at> gmail.com> writes:
>>
>>> OK, just add (derived-mode-p 'emacs-lisp-mode), as is done elsewhere
>>> in that file.
>>
>> I think we could do that, but I don't feel qualified to decide.  Maybe
>> Noam can help.
>
> I think it would be acceptable.

OK.  If noone opposes I will commit the patch after my sig in a couple
of days time.

>
>> As far as the example in your bug report is concerned, I think it would
>> also be an improvement if elisp-mode wouldn't try be clever in such a
>> way when indenting branches in a cond or variable associations in a let.
>
> The problem is that we currently don't have any way of specifying
> indentation for subforms of macro arguments, which is also the core
> problem of the other bugs I mentioned about indentation of cl-flet and
> friends.

Well, after some testing with

   (setq lisp-indent-function 'common-lisp-indent-function)

things seem to work as they should.  Though the name is "common lisp",
cl-indent.el's header says it can be used with emacs-lisp-mode, and
indeed it seems to be the case.  In Emacs -Q:

   (setq lisp-indent-function 'common-lisp-indent-function)
    
   (flet ((blablabla
              (correct)
            also-correct))
     ...)
    
   (defmacro deffoo (name args &rest body)
     ;; no indent spec needed
     `(defun ,name ,args ,@body))
    
   (deffoo test
       (correct)
     also-correct)
    
   (defmacro defbla (name args moreargs &rest body)
     (declare (indent 3))
     (frobnicate moreargs)
     `(defun ,name ,args ,@body))
    
   (defbla test
       (correct)
       (also-correct)
     also-also-correct)

   (cond (defoo
          correct))
    
   (let ((defoo
          correct)))

But I don't know if I'm missing anything very important here.  Are there
emacs-lisp indentation tests somewhere?

João

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index afb7cbd1dd..b1a99351ed 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -1063,8 +1063,8 @@ lisp-indent-function
 it specifies how to indent.  The property value can be:
 
 * `defun', meaning indent `defun'-style
-  (this is also the case if there is no property and the function
-  has a name that begins with \"def\", and three or more arguments);
+  (this is also the case if there is no property and a macro has
+  a name that begins with \"def\", and three or more arguments);
 
 * an integer N, meaning indent the first N arguments specially
   (like ordinary function arguments), and then indent any further
@@ -1104,7 +1104,9 @@ lisp-indent-function
 	(cond ((or (eq method 'defun)
 		   (and (null method)
 			(> (length function) 3)
-			(string-match "\\`def" function)))
+			(string-match "\\`def" function)
+                        (or (not (derived-mode-p 'emacs-lisp-mode))
+                            (macrop (intern function)))))
 	       (lisp-indent-defform state indent-point))
 	      ((integerp method)
 	       (lisp-indent-specform method state

	    

Message #42 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: João Távora <joaotavora <at> gmail.com>
Cc: 33301 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Sat, 10 Nov 2018 05:48:30 +0100

João Távora <joaotavora <at> gmail.com> writes:

> +                        (or (not (derived-mode-p 'emacs-lisp-mode))
> +                            (macrop (intern function)))))

That should better be `intern-soft' I think, right?

Michael.

Message #45 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 33301 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Sat, 10 Nov 2018 10:28:24 +0000

[Message part 1 (text/plain, inline)]

On Sat, Nov 10, 2018, 04:48 Michael Heerdegen <michael_heerdegen <at> web.de
wrote:

> João Távora <joaotavora <at> gmail.com> writes:
>
> > +                        (or (not (derived-mode-p 'emacs-lisp-mode))
> > +                            (macrop (intern function)))))
>
> That should better be `intern-soft' I think, right?
>

Probably, otherwise you intern some def-symbols even before reading then.
Hope nil doesn't break macrop.

And it has to ask for special-form-p, too otherwise your defvars are messed
up...

João

>
> Michael.
>

[Message part 2 (text/html, inline)]

Message #48 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Sat, 10 Nov 2018 11:54:46 +0100

On Nov 09 2018, João Távora <joaotavora <at> gmail.com> wrote:

> @@ -1104,7 +1104,9 @@ lisp-indent-function
>  	(cond ((or (eq method 'defun)
>  		   (and (null method)
>  			(> (length function) 3)
> -			(string-match "\\`def" function)))
> +			(string-match "\\`def" function)
> +                        (or (not (derived-mode-p 'emacs-lisp-mode))
> +                            (macrop (intern function)))))

Why is a defining function required to be a macro?

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Message #51 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Sat, 10 Nov 2018 12:46:18 +0000

Andreas Schwab <schwab <at> linux-m68k.org> writes:

> On Nov 09 2018, João Távora <joaotavora <at> gmail.com> wrote:
>
>> @@ -1104,7 +1104,9 @@ lisp-indent-function
>>  	(cond ((or (eq method 'defun)
>>  		   (and (null method)
>>  			(> (length function) 3)
>> -			(string-match "\\`def" function)))
>> +			(string-match "\\`def" function)
>> +                        (or (not (derived-mode-p 'emacs-lisp-mode))
>> +                            (macrop (intern function)))))
>
> Why is a defining function required to be a macro?

Do you know any that aren't?  Of course you can name a function however
you want, and give it your own "definition" semantics, but if you want
to do it in the lisp sense, generally the properties of macros are good:

1. They are evaluated at compilation time, so the byte compiler can
   understand the uses of your new definition down the line;

2. The arguments of a macro don't get evaluated unless you want them to,
   which is crucial for defining formal arguments.

João

PS: a defining function can also be a special form, which also has the
above properties

Message #54 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Sat, 10 Nov 2018 13:53:58 +0100

On Nov 10 2018, João Távora <joaotavora <at> gmail.com> wrote:

> Do you know any that aren't?

define-widget

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Message #57 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50;
 broken elisp indentation for non-definition symbols starting with
 "def.."
Date: Sat, 10 Nov 2018 16:05:44 +0000

Andreas Schwab <schwab <at> linux-m68k.org> writes:

> On Nov 10 2018, João Távora <joaotavora <at> gmail.com> wrote:
>
>> Do you know any that aren't?
>
> define-widget

A poorly chosen name for what should have been named `make-widget-type'
(and define-widget should have been a macro relieving the user of all
that useless quoting).

Anyhoo, it's a question of

  (function-put 'define-widget 'lisp-indent-function 'defun)

Any objections to that? Do you know any more?

João

Message #60 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Sat, 10 Nov 2018 16:18:25 +0000

[Message part 1 (text/plain, inline)]

Never mind  that, I just grepped for `(defun def` and there are a lot more,
though not quite as much as `defmacro def`

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.

Perhaps someone else can weigh in.

João

On Sat, Nov 10, 2018 at 4:05 PM João Távora <joaotavora <at> gmail.com> wrote:

> Andreas Schwab <schwab <at> linux-m68k.org> writes:
>
> > On Nov 10 2018, João Távora <joaotavora <at> gmail.com> wrote:
> >
> >> Do you know any that aren't?
> >
> > define-widget
>
> A poorly chosen name for what should have been named `make-widget-type'
> (and define-widget should have been a macro relieving the user of all
> that useless quoting).
>
> Anyhoo, it's a question of
>
>   (function-put 'define-widget 'lisp-indent-function 'defun)
>
> Any objections to that? Do you know any more?
>
> João
>
>
>
>
>
>

-- 
João Távora

[Message part 2 (text/html, inline)]

Message #63 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Sat, 22 Aug 2020 16:58:19 +0200

João Távora <joaotavora <at> gmail.com> writes:

> -			(string-match "\\`def" function)))
> +			(string-match "\\`def" function)
> +                        (or (not (derived-mode-p 'emacs-lisp-mode))
> +                            (macrop (intern function)))))

As others noted, this means that indentation changes when you've
loaded/not loaded the file that defines the macro, which may be
awkward.  But also, as your grep showed, there's oodles of functions in
Emacs the define stuff, and you'd have to add indentation specs to all
of them.  And that doesn't take out-of-tree definitions into account.

So I don't really see how this can be fixed in any sensible way --
changing this heuristic will just annoy more than it fixes things, I
think.

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

Message #66 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Sat, 22 Aug 2020 17:19:25 +0100

[Message part 1 (text/plain, inline)]

Hi Lars, thanks for your review of old bugs.

I wouldn't count 34 as "oodles" and don't think a new line for each
occurrence of what is essentially a breach of convention is a high price to
pay. Even converting some of those to macros or "make-foo" could be worth
it if it would enable non-surprising indentation.

As for the problem of needing to load macros before indenting forms where
they appears, that's already very much a thing. We wouldn't be creating new
problems there, it's just the way it is.

As for out-of-tree definitions, we could be lenient and have this saner
indentation be controlled by a variable which we would default to 'insane,
but to  'sane inside Emacs's source, via directory local variables.

So I don't think we should throw in the towel on this one.

João

On Sat, Aug 22, 2020, 15:58 Lars Ingebrigtsen <larsi <at> gnus.org> wrote:

> João Távora <joaotavora <at> gmail.com> writes:
>
> > -                     (string-match "\\`def" function)))
> > +                     (string-match "\\`def" function)
> > +                        (or (not (derived-mode-p 'emacs-lisp-mode))
> > +                            (macrop (intern function)))))
>
> As others noted, this means that indentation changes when you've
> loaded/not loaded the file that defines the macro, which may be
> awkward.  But also, as your grep showed, there's oodles of functions in
> Emacs the define stuff, and you'd have to add indentation specs to all
> of them.  And that doesn't take out-of-tree definitions into account.
>
> So I don't really see how this can be fixed in any sensible way --
> changing this heuristic will just annoy more than it fixes things, I
> think.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[Message part 2 (text/html, inline)]

Message #69 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Sun, 23 Aug 2020 14:26:46 +0200

João Távora <joaotavora <at> gmail.com> writes:

> I wouldn't count 34 as "oodles" and don't think a new line for each
> occurrence of what is essentially a breach of convention is a high
> price to pay. Even converting some of those to macros or "make-foo"
> could be worth it if it would enable non-surprising indentation.

Changing functions to macros, or renaming functions from def* to make*,
just because we have a slightly odd heuristic in Emacs Lisp mode doesn't
sound quite right to me.

> As for the problem of needing to load macros before indenting forms
> where they appears, that's already very much a thing. We wouldn't be
> creating new problems there, it's just the way it is.

That's true, but it would exacerbate the problem.  But the main problem
is that it would indent the code differently than it does now, and that
leads to whitespace churn in the vc, which we should avoid unless we
have a very, very good reason not to.  And this just doesn't seem like a
good enough reason...

> As for out-of-tree definitions, we could be lenient and have this
> saner indentation be controlled by a variable which we would default
> to 'insane, but to 'sane inside Emacs's source, via directory local
> variables.

I'd be against that -- again, because it leads to whitespace VC churn.

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

Message #72 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Sun, 23 Aug 2020 14:39:50 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>
>> I wouldn't count 34 as "oodles" and don't think a new line for each
>> occurrence of what is essentially a breach of convention is a high
>> price to pay. Even converting some of those to macros or "make-foo"
>> could be worth it if it would enable non-surprising indentation.
>
> Changing functions to macros, or renaming functions from def* to make*,
> just because we have a slightly odd heuristic in Emacs Lisp mode doesn't
> sound quite right to me.

This wouldn't be the only reason to do that.  As I explained
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33301#51 this convention
is good in itself.

>> As for the problem of needing to load macros before indenting forms
>> where they appears, that's already very much a thing. We wouldn't be
>> creating new problems there, it's just the way it is.
>
> That's true, but it would exacerbate the problem.

Really doubt that. I think regular use of macros (like, say with-foo) is
prevalent enough that people already know not to mass-reindent existing
code anyway.

> But the main problem is that it would indent the code differently than
> it does now, and that leads to whitespace churn in the vc, which we
> should avoid unless we have a very, very good reason not to.

What exactly do you mean "whitespace churn"?  Can you illustrate this
hypothetical scenario?  I don't expect whitespace/indentation beyond
fixing the akward cases, at least that's the entire point of this
report.

> good enough reason...

It is a somewhat infrequent but annoying bug when one decides to use a
variable name that happens to start with "def".  I use "default" and
"deferred" a lot, for instance.  In Common Lisp mode, I don't have this
problem.

>> As for out-of-tree definitions, we could be lenient and have this
>> saner indentation be controlled by a variable which we would default
>> to 'insane, but to 'sane inside Emacs's source, via directory local
>> variables.
>
> I'd be against that -- again, because it leads to whitespace VC churn.

Again, I'm missing something: this option wouldn't lead to that, I think

João

PS: another entirely different approach would just limit the current
hacky heuristic to calls/expansions that happen at top-level, i.e. at
"column 0".  I believe this to be the vast majority (though not the
entirety) of cases.

Message #75 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Mon, 24 Aug 2020 15:12:34 +0200

João Távora <joaotavora <at> gmail.com> writes:

>> But the main problem is that it would indent the code differently than
>> it does now, and that leads to whitespace churn in the vc, which we
>> should avoid unless we have a very, very good reason not to.
>
> What exactly do you mean "whitespace churn"?  Can you illustrate this
> hypothetical scenario?  I don't expect whitespace/indentation beyond
> fixing the akward cases, at least that's the entire point of this
> report.

It means indenting some things in a different way than today?  That
leads to whitespace changes.

>>> As for out-of-tree definitions, we could be lenient and have this
>>> saner indentation be controlled by a variable which we would default
>>> to 'insane, but to 'sane inside Emacs's source, via directory local
>>> variables.
>>
>> I'd be against that -- again, because it leads to whitespace VC churn.
>
> Again, I'm missing something: this option wouldn't lead to that, I think

If some people have the variable set to 'insane, they would indent the
code they're writing differently than the rest, which would lead to
whitespace churn.

> PS: another entirely different approach would just limit the current
> hacky heuristic to calls/expansions that happen at top-level, i.e. at
> "column 0".  I believe this to be the vast majority (though not the
> entirety) of cases.

That's probably true...

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

Message #78 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Tue, 25 Aug 2020 20:59:07 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> João Távora <joaotavora <at> gmail.com> writes:

> It means indenting some things in a different way than today?  That
> leads to whitespace changes.

Hmmm, an indentation bug such as this is, by definition, about an
incorrect amount of ... whitespace.  Right?

>>> I'd be against that -- again, because it leads to whitespace VC churn.
>> Again, I'm missing something: this option wouldn't lead to that, I think
> If some people have the variable set to 'insane, they would indent the
> code they're writing differently than the rest, which would lead to
> whitespace churn.

Well, they did set it to 'insane :-).  I don't see the problem here,
this variable would be similar to indent-tabs-mode.  If some people set
that differently it'll be equally disastrous.  But anyway, we probably
don't need the variable since I don't expect out of tree code to be
particularly affected: that's because, according to code conventions,
definitions should start with a package-specific prefix anyway.

João

Message #85 received at 33301 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 33301 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>, 43329 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33301: 27.0.50; broken elisp indentation for non-definition
 symbols starting with "def.."
Date: Mon, 18 Oct 2021 10:00:43 +0200

Lars Ingebrigtsen <larsi <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.