* bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
@ 2018-06-29 23:20 João Távora
2018-06-30 0:23 ` Noam Postavsky
0 siblings, 1 reply; 6+ messages in thread
From: João Távora @ 2018-06-29 23:20 UTC (permalink / raw)
To: 32014
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
2018-06-29 23:20 bug#32014: 26.1; lisp-indent-line fails in first line of Ielm João Távora
@ 2018-06-30 0:23 ` Noam Postavsky
2018-06-30 6:52 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Noam Postavsky @ 2018-06-30 0:23 UTC (permalink / raw)
To: João Távora; +Cc: 32014
[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]
tags 32014 + patch
quit
João Távora <joaotavora@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:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1222 bytes --]
From 2524780c54bcd8faecdb8497c0e1c960752fc9ce Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch --]
[-- Type: text/x-diff, Size: 2590 bytes --]
From 98e30ee9505f6e8cd21a36b88223e29ad9ee8281 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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
[-- Attachment #4: Type: text/plain, Size: 23 bytes --]
and this for master:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: patch --]
[-- Type: text/x-diff, Size: 1173 bytes --]
From 2dccc6d64669078915a7eda75f40e75408b7794e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
2018-06-30 0:23 ` Noam Postavsky
@ 2018-06-30 6:52 ` Eli Zaretskii
2018-06-30 13:27 ` Noam Postavsky
0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2018-06-30 6:52 UTC (permalink / raw)
To: Noam Postavsky; +Cc: joaotavora, 32014
> From: Noam Postavsky <npostavs@gmail.com>
> Date: Fri, 29 Jun 2018 20:23:13 -0400
> Cc: 32014@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
2018-06-30 6:52 ` Eli Zaretskii
@ 2018-06-30 13:27 ` Noam Postavsky
2018-07-10 0:48 ` Noam Postavsky
0 siblings, 1 reply; 6+ messages in thread
From: Noam Postavsky @ 2018-06-30 13:27 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: joaotavora, 32014
[-- Attachment #1: Type: text/plain, Size: 566 bytes --]
Eli Zaretskii <eliz@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:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1122 bytes --]
From a31918efdbdbf4c6d3f26ae7a73aba910f164116 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
2018-06-30 13:27 ` Noam Postavsky
@ 2018-07-10 0:48 ` Noam Postavsky
2018-07-10 7:04 ` João Távora
0 siblings, 1 reply; 6+ messages in thread
From: Noam Postavsky @ 2018-07-10 0:48 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: joaotavora, 32014
tags 32014 fixed
close 32014 26.2
quit
Noam Postavsky <npostavs@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#32014: 26.1; lisp-indent-line fails in first line of Ielm
2018-07-10 0:48 ` Noam Postavsky
@ 2018-07-10 7:04 ` João Távora
0 siblings, 0 replies; 6+ messages in thread
From: João Távora @ 2018-07-10 7:04 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 32014
[-- Attachment #1: Type: text/plain, Size: 148 bytes --]
On Tue, Jul 10, 2018, 01:48 Noam Postavsky <npostavs@gmail.com> wrote:
> tags 32014 fixed
> close 32014 26.2
> quit
>
Thanks!
João
>
[-- Attachment #2: Type: text/html, Size: 626 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-10 7:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-29 23:20 bug#32014: 26.1; lisp-indent-line fails in first line of Ielm João Távora
2018-06-30 0:23 ` Noam Postavsky
2018-06-30 6:52 ` Eli Zaretskii
2018-06-30 13:27 ` Noam Postavsky
2018-07-10 0:48 ` Noam Postavsky
2018-07-10 7:04 ` João Távora
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.