* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." @ 2018-11-07 13:21 João Távora 2018-11-08 0:05 ` Noam Postavsky 0 siblings, 1 reply; 24+ messages in thread From: João Távora @ 2018-11-07 13:21 UTC (permalink / raw) To: 33301 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-07 13:21 bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." João Távora @ 2018-11-08 0:05 ` Noam Postavsky 2018-11-08 0:35 ` Michael Heerdegen 0 siblings, 1 reply; 24+ messages in thread From: Noam Postavsky @ 2018-11-08 0:05 UTC (permalink / raw) To: João Távora; +Cc: 33301 found 33301 24.3 tags 33301 + confirmed quit João Távora <joaotavora@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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-08 0:05 ` Noam Postavsky @ 2018-11-08 0:35 ` Michael Heerdegen 2018-11-08 9:52 ` João Távora 0 siblings, 1 reply; 24+ messages in thread From: Michael Heerdegen @ 2018-11-08 0:35 UTC (permalink / raw) To: Noam Postavsky; +Cc: 33301, João Távora Noam Postavsky <npostavs@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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-08 0:35 ` Michael Heerdegen @ 2018-11-08 9:52 ` João Távora 2018-11-09 0:13 ` Michael Heerdegen 0 siblings, 1 reply; 24+ messages in thread From: João Távora @ 2018-11-08 9:52 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 33301, Noam Postavsky Michael Heerdegen <michael_heerdegen@web.de> writes: > Noam Postavsky <npostavs@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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-08 9:52 ` João Távora @ 2018-11-09 0:13 ` Michael Heerdegen 2018-11-09 0:41 ` João Távora 0 siblings, 1 reply; 24+ messages in thread From: Michael Heerdegen @ 2018-11-09 0:13 UTC (permalink / raw) To: João Távora; +Cc: 33301, Noam Postavsky João Távora <joaotavora@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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 0:13 ` Michael Heerdegen @ 2018-11-09 0:41 ` João Távora 2018-11-09 1:45 ` Michael Heerdegen 0 siblings, 1 reply; 24+ messages in thread From: João Távora @ 2018-11-09 0:41 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 33301, Noam Postavsky Michael Heerdegen <michael_heerdegen@web.de> writes: > João Távora <joaotavora@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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 0:41 ` João Távora @ 2018-11-09 1:45 ` Michael Heerdegen 2018-11-09 9:04 ` João Távora 0 siblings, 1 reply; 24+ messages in thread From: Michael Heerdegen @ 2018-11-09 1:45 UTC (permalink / raw) To: João Távora; +Cc: 33301, Noam Postavsky João Távora <joaotavora@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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 1:45 ` Michael Heerdegen @ 2018-11-09 9:04 ` João Távora 2018-11-09 9:51 ` Michael Heerdegen 0 siblings, 1 reply; 24+ messages in thread From: João Távora @ 2018-11-09 9:04 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 33301, Noam Postavsky Michael Heerdegen <michael_heerdegen@web.de> writes: > João Távora <joaotavora@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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 9:04 ` João Távora @ 2018-11-09 9:51 ` Michael Heerdegen 2018-11-09 12:28 ` Noam Postavsky 0 siblings, 1 reply; 24+ messages in thread From: Michael Heerdegen @ 2018-11-09 9:51 UTC (permalink / raw) To: João Távora; +Cc: 33301, Noam Postavsky João Távora <joaotavora@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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 9:51 ` Michael Heerdegen @ 2018-11-09 12:28 ` Noam Postavsky 2018-11-09 19:39 ` João Távora 0 siblings, 1 reply; 24+ messages in thread From: Noam Postavsky @ 2018-11-09 12:28 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 33301, João Távora Michael Heerdegen <michael_heerdegen@web.de> writes: > João Távora <joaotavora@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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 12:28 ` Noam Postavsky @ 2018-11-09 19:39 ` João Távora 2018-11-10 4:48 ` Michael Heerdegen ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: João Távora @ 2018-11-09 19:39 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Heerdegen, 33301 Noam Postavsky <npostavs@gmail.com> writes: > Michael Heerdegen <michael_heerdegen@web.de> writes: > >> João Távora <joaotavora@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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 19:39 ` João Távora @ 2018-11-10 4:48 ` Michael Heerdegen 2018-11-10 10:28 ` João Távora 2018-11-10 10:54 ` Andreas Schwab 2020-08-22 14:58 ` Lars Ingebrigtsen 2 siblings, 1 reply; 24+ messages in thread From: Michael Heerdegen @ 2018-11-10 4:48 UTC (permalink / raw) To: João Távora; +Cc: 33301, Noam Postavsky João Távora <joaotavora@gmail.com> writes: > + (or (not (derived-mode-p 'emacs-lisp-mode)) > + (macrop (intern function))))) That should better be `intern-soft' I think, right? Michael. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-10 4:48 ` Michael Heerdegen @ 2018-11-10 10:28 ` João Távora 0 siblings, 0 replies; 24+ messages in thread From: João Távora @ 2018-11-10 10:28 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 33301, Noam Postavsky [-- Attachment #1: Type: text/plain, Size: 557 bytes --] On Sat, Nov 10, 2018, 04:48 Michael Heerdegen <michael_heerdegen@web.de wrote: > João Távora <joaotavora@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. > [-- Attachment #2: Type: text/html, Size: 1241 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 19:39 ` João Távora 2018-11-10 4:48 ` Michael Heerdegen @ 2018-11-10 10:54 ` Andreas Schwab 2018-11-10 12:46 ` João Távora 2020-08-22 14:58 ` Lars Ingebrigtsen 2 siblings, 1 reply; 24+ messages in thread From: Andreas Schwab @ 2018-11-10 10:54 UTC (permalink / raw) To: João Távora; +Cc: Michael Heerdegen, 33301, Noam Postavsky On Nov 09 2018, João Távora <joaotavora@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@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-10 10:54 ` Andreas Schwab @ 2018-11-10 12:46 ` João Távora 2018-11-10 12:53 ` Andreas Schwab 0 siblings, 1 reply; 24+ messages in thread From: João Távora @ 2018-11-10 12:46 UTC (permalink / raw) To: Andreas Schwab; +Cc: Michael Heerdegen, 33301, Noam Postavsky Andreas Schwab <schwab@linux-m68k.org> writes: > On Nov 09 2018, João Távora <joaotavora@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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-10 12:46 ` João Távora @ 2018-11-10 12:53 ` Andreas Schwab 2018-11-10 16:05 ` João Távora 0 siblings, 1 reply; 24+ messages in thread From: Andreas Schwab @ 2018-11-10 12:53 UTC (permalink / raw) To: João Távora; +Cc: Michael Heerdegen, 33301, Noam Postavsky On Nov 10 2018, João Távora <joaotavora@gmail.com> wrote: > Do you know any that aren't? define-widget Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-10 12:53 ` Andreas Schwab @ 2018-11-10 16:05 ` João Távora 2018-11-10 16:18 ` João Távora 0 siblings, 1 reply; 24+ messages in thread From: João Távora @ 2018-11-10 16:05 UTC (permalink / raw) To: Andreas Schwab; +Cc: Michael Heerdegen, 33301, Noam Postavsky Andreas Schwab <schwab@linux-m68k.org> writes: > On Nov 10 2018, João Távora <joaotavora@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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-10 16:05 ` João Távora @ 2018-11-10 16:18 ` João Távora 0 siblings, 0 replies; 24+ messages in thread From: João Távora @ 2018-11-10 16:18 UTC (permalink / raw) To: Andreas Schwab; +Cc: Michael Heerdegen, 33301, Noam Postavsky [-- Attachment #1: Type: text/plain, Size: 994 bytes --] 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@gmail.com> wrote: > Andreas Schwab <schwab@linux-m68k.org> writes: > > > On Nov 10 2018, João Távora <joaotavora@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 [-- Attachment #2: Type: text/html, Size: 1683 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2018-11-09 19:39 ` João Távora 2018-11-10 4:48 ` Michael Heerdegen 2018-11-10 10:54 ` Andreas Schwab @ 2020-08-22 14:58 ` Lars Ingebrigtsen 2020-08-22 16:19 ` João Távora 2 siblings, 1 reply; 24+ messages in thread From: Lars Ingebrigtsen @ 2020-08-22 14:58 UTC (permalink / raw) To: João Távora; +Cc: Michael Heerdegen, 33301, Noam Postavsky João Távora <joaotavora@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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2020-08-22 14:58 ` Lars Ingebrigtsen @ 2020-08-22 16:19 ` João Távora 2020-08-23 12:26 ` Lars Ingebrigtsen 0 siblings, 1 reply; 24+ messages in thread From: João Távora @ 2020-08-22 16:19 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Michael Heerdegen, 33301, Noam Postavsky [-- Attachment #1: Type: text/plain, Size: 1825 bytes --] 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@gnus.org> wrote: > João Távora <joaotavora@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 > [-- Attachment #2: Type: text/html, Size: 2690 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2020-08-22 16:19 ` João Távora @ 2020-08-23 12:26 ` Lars Ingebrigtsen 2020-08-23 13:39 ` João Távora 0 siblings, 1 reply; 24+ messages in thread From: Lars Ingebrigtsen @ 2020-08-23 12:26 UTC (permalink / raw) To: João Távora; +Cc: Michael Heerdegen, 33301, Noam Postavsky João Távora <joaotavora@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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2020-08-23 12:26 ` Lars Ingebrigtsen @ 2020-08-23 13:39 ` João Távora 2020-08-24 13:12 ` Lars Ingebrigtsen 0 siblings, 1 reply; 24+ messages in thread From: João Távora @ 2020-08-23 13:39 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Michael Heerdegen, 33301, Noam Postavsky Lars Ingebrigtsen <larsi@gnus.org> writes: > João Távora <joaotavora@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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2020-08-23 13:39 ` João Távora @ 2020-08-24 13:12 ` Lars Ingebrigtsen 2020-08-25 19:59 ` João Távora 0 siblings, 1 reply; 24+ messages in thread From: Lars Ingebrigtsen @ 2020-08-24 13:12 UTC (permalink / raw) To: João Távora; +Cc: Michael Heerdegen, 33301, Noam Postavsky João Távora <joaotavora@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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." 2020-08-24 13:12 ` Lars Ingebrigtsen @ 2020-08-25 19:59 ` João Távora 0 siblings, 0 replies; 24+ messages in thread From: João Távora @ 2020-08-25 19:59 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Michael Heerdegen, 33301, Noam Postavsky Lars Ingebrigtsen <larsi@gnus.org> writes: > João Távora <joaotavora@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 ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-08-25 19:59 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-07 13:21 bug#33301: 27.0.50; broken elisp indentation for non-definition symbols starting with "def.." João Távora 2018-11-08 0:05 ` Noam Postavsky 2018-11-08 0:35 ` Michael Heerdegen 2018-11-08 9:52 ` João Távora 2018-11-09 0:13 ` Michael Heerdegen 2018-11-09 0:41 ` João Távora 2018-11-09 1:45 ` Michael Heerdegen 2018-11-09 9:04 ` João Távora 2018-11-09 9:51 ` Michael Heerdegen 2018-11-09 12:28 ` Noam Postavsky 2018-11-09 19:39 ` João Távora 2018-11-10 4:48 ` Michael Heerdegen 2018-11-10 10:28 ` João Távora 2018-11-10 10:54 ` Andreas Schwab 2018-11-10 12:46 ` João Távora 2018-11-10 12:53 ` Andreas Schwab 2018-11-10 16:05 ` João Távora 2018-11-10 16:18 ` João Távora 2020-08-22 14:58 ` Lars Ingebrigtsen 2020-08-22 16:19 ` João Távora 2020-08-23 12:26 ` Lars Ingebrigtsen 2020-08-23 13:39 ` João Távora 2020-08-24 13:12 ` Lars Ingebrigtsen 2020-08-25 19:59 ` João Távora
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).