procyberian/emacs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Finish excising electric indent from `open-line'

Browse source 
* lisp/simple.el (open-line): Remove INTERACTIVE argument.

* test/automated/simple-test.el (open-line-indent, open-line-hook):
  Adjust accordingly.

This change finishes what my commit of Thu Nov 19 17:32:37 2015 -0600
(git commit c59353896) started.  It turns out that having INTERACTIVE
cause `post-self-insert-hook' to run (via `newline') meant `open-line'
still had the electric indent behavior, as `post-self-insert-hook'
normally contains `electric-indent-post-self-insert-function' ever
since `electric-indent-mode' has been on by default.  Tracing the code
change in `open-line' is mildly twisty, because Artur Malabarba's
earliest two commits of 24 Oct 2015 first removed the `interactive'
form entirely (git commit 6939896e2) and then restored it with the new
extra "p" already added (git commit bd4f04f86), such that there is no
single-commit diff in which one sees the second "p" appear.  Thus this
change is effectively a reversion of parts of each of those commits.

This could close bug#21884, at least until further discussion.
This commit is contained in:
Karl Fogel 2015-11-21 22:50:05 -06:00
parent 51fd4a0139
commit 8726de6663
2 changed files with 24 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
lisp/simple.el
View file

@ -458,19 +458,18 @@ A non-nil INTERACTIVE argument means to run the `post-self-insert-hook'."
(put-text-property from (point) 'rear-nonsticky
(cons 'hard sticky)))))
(defun open-line (n &optional interactive)
(defun open-line (n)
"Insert a newline and leave point before it.
If there is a fill prefix and/or a `left-margin', insert them on
the new line if the line would have been blank.
With arg N, insert N newlines.
A non-nil INTERACTIVE argument means to run the `post-self-insert-hook'."
(interactive "*p\np")
With arg N, insert N newlines."
(interactive "*p")
(let* ((do-fill-prefix (and fill-prefix (bolp)))
(do-left-margin (and (bolp) (> (current-left-margin) 0)))
(loc (point-marker))
;; Don't expand an abbrev before point.
(abbrev-mode nil))
(newline n interactive)
(newline n)
(goto-char loc)
(while (> n 0)
(cond ((bolp)

44
test/automated/simple-test.el
View file

@ -138,21 +138,12 @@
(open-line 1)))
'("- - " . "\n(a b c d)"))))
;; For a while, from 24 Oct - 19 Nov 2015, `open-line' in the Emacs
;; For a while, from 24 Oct - 21 Nov 2015, `open-line' in the Emacs
;; development tree became sensitive to `electric-indent-mode', which
;; it had not been before. This sensitivity was reverted for the
;; Emacs 25 release, so it could be discussed further (see thread
;; "Questioning the new behavior of `open-line'." on the Emacs Devel
;; mailing list). The only test case here that started failing after
;; the reversion is the third one, the one that currently expects
;; `("(a b" . "\n \n c d)")'. If `open-line' were again sensitive
;; to electric indent, then the three spaces between the two newlines
;; would go away, leaving `("(a b" . "\n\n c d)")'.
;;
;; If electric indent sensitivity were re-enabled, we might also want
;; to make the test cases below a bit stricter, or add some more test
;; cases that are specific to `electric-indent-mode', since right now
;; all but one of the cases pass with or without electric indent.
;; mailing list, and bug #21884).
(ert-deftest open-line-indent ()
(should (equal (simple-test--dummy-buffer
(electric-indent-local-mode 1)
@ -160,29 +151,34 @@
'("(a b" . "\n c d)")))
(should (equal (simple-test--dummy-buffer
(electric-indent-local-mode 1)
(open-line 1 'interactive))
'("(a b" . "\n c d)")))
(open-line 1))
'("(a b" . "\n c d)")))
(should (equal (simple-test--dummy-buffer
(electric-indent-local-mode 1)
(let ((current-prefix-arg nil))
(call-interactively #'open-line)
(call-interactively #'open-line)))
'("(a b" . "\n \n c d)")))
'("(a b" . "\n\n c d)")))
(should (equal (simple-test--dummy-buffer
(electric-indent-local-mode 1)
(open-line 5 'interactive))
'("(a b" . "\n\n\n\n\n c d)")))
(open-line 5))
'("(a b" . "\n\n\n\n\n c d)")))
(should (equal (simple-test--dummy-buffer
(electric-indent-local-mode 1)
(let ((current-prefix-arg 5))
(call-interactively #'open-line)))
'("(a b" . "\n\n\n\n\n c d)")))
'("(a b" . "\n\n\n\n\n c d)")))
(should (equal (simple-test--dummy-buffer
(forward-char 1)
(electric-indent-local-mode 1)
(open-line 1 'interactive))
'("(a b" . "\n c d)"))))
(open-line 1))
'("(a b " . "\nc d)"))))
;; From 24 Oct - 21 Nov 2015, `open-line' took a second argument
;; INTERACTIVE and ran `post-self-insert-hook' if the argument was
;; true. This test tested that. Currently, however, `open-line'
;; does not run run `post-self-insert-hook' at all, so for now
;; this test just makes sure that it doesn't.
(ert-deftest open-line-hook ()
(let* ((x 0)
(inc (lambda () (setq x (1+ x)))))
@ -192,18 +188,18 @@
(should (= x 0))
(simple-test--dummy-buffer
(add-hook 'post-self-insert-hook inc nil 'local)
(open-line 1 'interactive))
(should (= x 1))
(open-line 1))
(should (= x 0))
(unwind-protect
(progn
(add-hook 'post-self-insert-hook inc)
(simple-test--dummy-buffer
(open-line 1))
(should (= x 1))
(should (= x 0))
(simple-test--dummy-buffer
(open-line 10 'interactive))
(should (= x 2)))
(open-line 10))
(should (= x 0)))
(remove-hook 'post-self-insert-hook inc))))