GNU bug report logs - #32014
26.1; lisp-indent-line fails in first line of Ielm

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 32014 in the body.
You can then email your comments to 32014 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: 26.1; lisp-indent-line fails in first line of Ielm
Date: Sat, 30 Jun 2018 00:20:11 +0100

Hi maintainers,

Originally reported in https://github.com/joaotavora/sly/issues/165, for
SLY, a Common Lisp IDE with a REPL similar to Ielm's

  Emacs -Q
  M-x ielm
  SPC f o o
  M-x lisp-indent-line

Point is moved to the beginning of the line, and an error is signalled.

The error's backtrace isn't shown even with debug-on-error set to t.

Is this the intended behaviour?  In 25.2 the string foo was correctly
indented back one character, so this seems like a regression.

If I do M-x lisp-indent-line in any line but the first of a multi-line
expression, it works nicely.

João

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 32014 <at> debbugs.gnu.org
Subject: Re: bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
Date: Fri, 29 Jun 2018 20:23:13 -0400

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

tags 32014 + patch
quit

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

> The error's backtrace isn't shown even with debug-on-error set to t.

If you (setq debug-ignored-errors nil) first, then the backtrace is

Debugger entered--Lisp error: (text-read-only)
  indent-line-to(7)
  lisp-indent-line()
  funcall-interactively(lisp-indent-line)
  call-interactively(lisp-indent-line record nil)
  command-execute(lisp-indent-line record)
  execute-extended-command(nil "lisp-indent-line" "lisp-indent-line")
  funcall-interactively(execute-extended-command nil "lisp-indent-line" "lisp-indent-line")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

> Is this the intended behaviour?  In 25.2 the string foo was correctly
> indented back one character, so this seems like a regression.

No, it's an accident.  In lisp-indent-line, I simplified 

	(setq shift-amt (- indent (current-column)))
	(if (zerop shift-amt)
	    nil
	  (delete-region beg (point))
	  (indent-to indent)))

into

    (indent-line-to indent)

but it turns out not be equivalent in this case.  indent-line-to doesn't
respect the prompt's field property.

I propose this for emacs-26:

[v1-0001-Test-for-Bug-32014.patch (text/x-diff, inline)]

From 2524780c54bcd8faecdb8497c0e1c960752fc9ce Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Fri, 29 Jun 2018 20:15:10 -0400
Subject: [PATCH v1 1/2] ; Test for Bug#32014

* test/lisp/emacs-lisp/lisp-mode-tests.el
(lisp-indent-with-read-only-field): New test.
---
 test/lisp/emacs-lisp/lisp-mode-tests.el | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el
index 0b5b0a4019..2ac0e5ce1d 100644
--- a/test/lisp/emacs-lisp/lisp-mode-tests.el
+++ b/test/lisp/emacs-lisp/lisp-mode-tests.el
@@ -224,6 +224,17 @@ lisp-mode-tests--correctly-indented-sexp
       (comment-indent)
       (should (equal (buffer-string) correct)))))
 
+(ert-deftest lisp-indent-with-read-only-field ()
+  "Test indentation on line with read-only field (Bug#32014)."
+  :expected-result :failed
+  (with-temp-buffer
+    (insert (propertize "prompt> " 'field 'output 'read-only t
+                        'rear-nonsticky t 'front-sticky '(read-only)))
+    (insert " foo")
+    (lisp-indent-line)
+    (should (equal (buffer-string) "prompt> foo"))))
+
+
 
 (provide 'lisp-mode-tests)
 ;;; lisp-mode-tests.el ends here
-- 
2.11.0

[v1-0002-Stop-using-indent-line-to-in-lisp-indent-line-Bug.patch (text/x-diff, inline)]

From 98e30ee9505f6e8cd21a36b88223e29ad9ee8281 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Fri, 29 Jun 2018 19:58:58 -0400
Subject: [PATCH v1 2/2] Stop using indent-line-to in lisp-indent-line
 (Bug#32014)

This is partial revert of "Remove ignored argument from
lisp-indent-line", because `indent-line-to' doesn't respect field
boundaries.
* lisp/emacs-lisp/lisp-mode.el (lisp-indent-line): Use delete-region
and indent-to instead of `indent-line-to'.
* test/lisp/emacs-lisp/lisp-mode-tests.el
(lisp-indent-with-read-only-field): Expect to pass.

Don't merge to master, we will fix indent-line-to there instead.
---
 lisp/emacs-lisp/lisp-mode.el            | 10 ++++++++--
 test/lisp/emacs-lisp/lisp-mode-tests.el |  1 -
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index 94be5acd6d..3a03b56313 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -867,7 +867,9 @@ lisp-indent-line
   (interactive)
   (let ((pos (- (point-max) (point)))
         (indent (progn (beginning-of-line)
-                       (or indent (calculate-lisp-indent (lisp-ppss))))))
+                       (or indent (calculate-lisp-indent (lisp-ppss)))))
+	(shift-amt nil)
+	(beg (progn (beginning-of-line) (point))))
     (skip-chars-forward " \t")
     (if (or (null indent) (looking-at "\\s<\\s<\\s<"))
 	;; Don't alter indentation of a ;;; comment line
@@ -879,7 +881,11 @@ lisp-indent-line
 	  ;; as comment lines, not as code.
 	  (progn (indent-for-comment) (forward-char -1))
 	(if (listp indent) (setq indent (car indent)))
-        (indent-line-to indent))
+	(setq shift-amt (- indent (current-column)))
+	(if (zerop shift-amt)
+	    nil
+	  (delete-region beg (point))
+	  (indent-to indent)))
       ;; If initial point was within line's indentation,
       ;; position after the indentation.  Else stay at same point in text.
       (if (> (- (point-max) pos) (point))
diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el
index 2ac0e5ce1d..8598d41978 100644
--- a/test/lisp/emacs-lisp/lisp-mode-tests.el
+++ b/test/lisp/emacs-lisp/lisp-mode-tests.el
@@ -226,7 +226,6 @@ lisp-mode-tests--correctly-indented-sexp
 
 (ert-deftest lisp-indent-with-read-only-field ()
   "Test indentation on line with read-only field (Bug#32014)."
-  :expected-result :failed
   (with-temp-buffer
     (insert (propertize "prompt> " 'field 'output 'read-only t
                         'rear-nonsticky t 'front-sticky '(read-only)))
-- 
2.11.0

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

and this for master:

[0001-Respect-field-boundaries-in-to-indentation-functions.patch (text/x-diff, inline)]

From 2dccc6d64669078915a7eda75f40e75408b7794e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Fri, 29 Jun 2018 20:01:53 -0400
Subject: [PATCH] Respect field boundaries in *-to-indentation functions
 (Bug#32014)

* lisp/simple.el (forward-to-indentation)
(backward-to-indentation): Use `beginning-of-line' which respects
field boundaries rather than `forward-line' which doesn't.
---
 lisp/simple.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index f8c02c1dbf..3cece52657 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -872,13 +872,13 @@ quoted-insert
 (defun forward-to-indentation (&optional arg)
   "Move forward ARG lines and position at first nonblank character."
   (interactive "^p")
-  (forward-line (or arg 1))
+  (beginning-of-line (+ 1 (or arg 1)))
   (skip-chars-forward " \t"))
 
 (defun backward-to-indentation (&optional arg)
   "Move backward ARG lines and position at first nonblank character."
   (interactive "^p")
-  (forward-line (- (or arg 1)))
+  (beginning-of-line (- 1 (or arg 1)))
   (skip-chars-forward " \t"))
 
 (defun back-to-indentation ()
-- 
2.11.0

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: joaotavora <at> gmail.com, 32014 <at> debbugs.gnu.org
Subject: Re: bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
Date: Sat, 30 Jun 2018 09:52:02 +0300

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Fri, 29 Jun 2018 20:23:13 -0400
> Cc: 32014 <at> debbugs.gnu.org
> 
> I propose this for emacs-26:

OK.

> and this for master:

I'm not sure I'm okay with changing the behavior of
forward/backward-to-indentation like that.  It's an incompatible
change, isn't it?  The documentation doesn't seem to tell anything wrt
the behavior in presence of fields, but that doesn't mean we can make
such changes without considering the consequences.  Can you tell why
you think this change is TRT?

In any case, such a change should be reflected in NEWS and in the doc
strings.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: joaotavora <at> gmail.com, 32014 <at> debbugs.gnu.org
Subject: Re: bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
Date: Sat, 30 Jun 2018 09:27:58 -0400

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

Eli Zaretskii <eliz <at> gnu.org> writes:

> I'm not sure I'm okay with changing the behavior of
> forward/backward-to-indentation like that.  It's an incompatible
> change, isn't it?  The documentation doesn't seem to tell anything wrt
> the behavior in presence of fields, but that doesn't mean we can make
> such changes without considering the consequences.  Can you tell why
> you think this change is TRT?

Hmm, the more I look at it, the less I understand why these functions
even exist.  There are hardly any uses of them.  Maybe we should just do
this instead:

[v2-0001-Respect-field-boundaries-in-indent-line-to-Bug-32.patch (text/x-diff, inline)]

From a31918efdbdbf4c6d3f26ae7a73aba910f164116 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sat, 30 Jun 2018 09:14:22 -0400
Subject: [PATCH v2] Respect field boundaries in indent-line-to (Bug#32014)

* lisp/indent.el (indent-line-to): Use the back-to-indentation point
as the end-point of whitespace removal, rather than
backward-to-indentation which doesn't respect field boundaries.
---
 lisp/indent.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/indent.el b/lisp/indent.el
index eb5b21e8e8..14efe8bfdf 100644
--- a/lisp/indent.el
+++ b/lisp/indent.el
@@ -300,8 +300,9 @@ indent-line-to
 			      (progn (skip-chars-backward " ") (point))))
 	   (indent-to column))
 	  ((> cur-col column) ; too far right (after tab?)
-	   (delete-region (progn (move-to-column column t) (point))
-			  (progn (backward-to-indentation 0) (point)))))))
+	   (let ((cur-indent (point)))
+             (delete-region (progn (move-to-column column t) (point))
+			    cur-indent))))))
 
 (defun current-left-margin ()
   "Return the left margin to use for this line.
-- 
2.11.0

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: joaotavora <at> gmail.com, 32014 <at> debbugs.gnu.org
Subject: Re: bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
Date: Mon, 09 Jul 2018 20:48:35 -0400

tags 32014 fixed
close 32014 26.2
quit

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

> Hmm, the more I look at it, the less I understand why these functions
> even exist.  There are hardly any uses of them.  Maybe we should just do
> this instead:

> * lisp/indent.el (indent-line-to): Use the back-to-indentation point
> as the end-point of whitespace removal, rather than
> backward-to-indentation which doesn't respect field boundaries.

Pushed this to master [3: e4ad2d1a8f] and the other fixes to emacs-26
[1: db3f779780] [2: 8f7d35cabd].

[1: db3f779780]: 2018-07-09 19:39:03 -0400
  ; Test for Bug#32014
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=db3f7797809ed9de8dd92ce38bf34f768ddc64ad

[2: 8f7d35cabd]: 2018-07-09 19:39:03 -0400
  Stop using indent-line-to in lisp-indent-line (Bug#32014)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=8f7d35cabdbeb2404d53af39c5d7c12e870fa1cb

[3: e4ad2d1a8f]: 2018-07-09 20:08:13 -0400
  Respect field boundaries in indent-line-to (Bug#32014)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e4ad2d1a8fad8c8c786b61083b05cfaa1ea5669c

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

From: João Távora <joaotavora <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 32014 <at> debbugs.gnu.org
Subject: Re: bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
Date: Tue, 10 Jul 2018 08:04:26 +0100

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

On Tue, Jul 10, 2018, 01:48 Noam Postavsky <npostavs <at> gmail.com> wrote:

> tags 32014 fixed
> close 32014 26.2
> quit
>

Thanks!
João

>

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

Previous Next

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