From 8726de6663608b74e81ff88e530b4ddc6165700f Mon Sep 17 00:00:00 2001 From: Karl Fogel Date: Sat, 21 Nov 2015 22:50:05 -0600 Subject: [PATCH] Finish excising electric indent from `open-line' * 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. --- lisp/simple.el | 9 ++++--- test/automated/simple-test.el | 44 ++++++++++++++++------------------- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/lisp/simple.el b/lisp/simple.el index 8b57bf04bea..6a745c7cb25 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -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) diff --git a/test/automated/simple-test.el b/test/automated/simple-test.el index 7e0dbb786a7..a3931ef9277 100644 --- a/test/automated/simple-test.el +++ b/test/automated/simple-test.el @@ -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)))) -- 2.39.5