From fd943124439b7644392919bca8bc2a77e6316d92 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jo=C3=A3o=20T=C3=A1vora?= Date: Tue, 22 Jan 2019 15:46:56 +0000 Subject: [PATCH] electric-layout-mode kicks in before electric-pair-mode This aims to solve problems with indentation. Previously in, say, a js-mode buffer with electric-layout-rules set to (?\{ before after) (?\} before) would produce an intended: function () { } The initial state function () { Would go immediately to the following by e-p-m function () {} Only then would e-l-m be applied to } first, and then again to {. This makes lines indent in the wrong order, which can be a problem in some modes. The way we fix this is by reversing the order of e-p-m and e-l-m in the post-self-insert-hook (and also fixing a number of details that this uncovered). In the end this changes the sequence from function () { By way of e-l-m becomes: function () { The e-p-m inserts the pair function () { } And then e-l-m kicks in for the pair again, yielding the desired result function () { } * lisp/elec-pair.el (electric-pair--insert): Bind electric-layout-no-duplicate-newlines. (electric-pair-inhibit-if-helps-balance) (electric-pair-skip-if-helps-balance): Use insert-before-markers, playing nice with save-excurion. (electric-pair-post-self-insert-function): Go to correct position before checking electric-pair-inhibit-predicate and electric-pair-skip-self predicate. (electric-pair-post-self-insert-function): Increase priority to 50. * lisp/electric.el (electric-indent-post-self-insert-function): Delete trailing space in reindented line only if line was really reindented. Rewrite comment. (electric-layout-allow-duplicate-newlines): New variable. (electric-layout-post-self-insert-function-1): Rewrite comments. Honours electric-layout-allow-duplicate-newlines. Don't reindent previous line because racecar. * test/lisp/electric-tests.el: New test. (plainer-c-mode): Move up. (electric-modes-int-main-allman-style) (electric-layout-int-main-kernel-style): Simplify electric-layout-rules. (electric-layout-for-c-style-du-jour): New helper. (electric-layout-plainer-c-mode-use-c-style): New test. --- lisp/elec-pair.el | 20 ++++++---- lisp/electric.el | 76 +++++++++++++++++++++++++------------ test/lisp/electric-tests.el | 57 ++++++++++++++++++++++++---- 3 files changed, 113 insertions(+), 40 deletions(-) diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el index 2c5553e8dab..20581ad6573 100644 --- a/lisp/elec-pair.el +++ b/lisp/elec-pair.el @@ -227,7 +227,8 @@ inside a comment or string." (defun electric-pair--insert (char) (let ((last-command-event char) (blink-matching-paren nil) - (electric-pair-mode nil)) + (electric-pair-mode nil) + (electric-layout-allow-duplicate-newlines t)) (self-insert-command 1))) (cl-defmacro electric-pair--with-uncached-syntax ((table &optional start) &rest body) @@ -426,11 +427,10 @@ happened." (eq (cdr outermost) pair))))) ((eq syntax ?\") (electric-pair--unbalanced-strings-p char)))) - (insert-char char))))) + (insert-before-markers char))))) (defun electric-pair-skip-if-helps-balance (char) "Return non-nil if skipping CHAR would benefit parentheses' balance. - Works by first removing the character from the buffer, then doing some list calculations, finally restoring the situation as if nothing happened." @@ -452,7 +452,7 @@ happened." (not (eq (cdr outermost) pair))))))) ((eq syntax ?\") (electric-pair--inside-string-p char)))) - (insert-char char))))) + (insert-before-markers char))))) (defun electric-pair-default-skip-self (char) (if electric-pair-preserve-balance @@ -498,7 +498,9 @@ happened." ((and (memq syntax '(?\) ?\" ?\$)) (and (or unconditional (if (functionp electric-pair-skip-self) - (funcall electric-pair-skip-self last-command-event) + (save-excursion + (goto-char pos) + (funcall electric-pair-skip-self last-command-event)) electric-pair-skip-self)) (save-excursion (when (and (not (and unconditional @@ -525,8 +527,10 @@ happened." ((and (memq syntax '(?\( ?\" ?\$)) (not overwrite-mode) (or unconditional - (not (funcall electric-pair-inhibit-predicate - last-command-event)))) + (not (save-excursion + (goto-char pos) + (funcall electric-pair-inhibit-predicate + last-command-event))))) (save-excursion (electric-pair--insert pair))))) (_ (when (and (if (functionp electric-pair-open-newline-between-pairs) @@ -540,7 +544,7 @@ happened." (matching-paren (char-after)))) (save-excursion (newline 1 t))))))) -(put 'electric-pair-post-self-insert-function 'priority 20) +(put 'electric-pair-post-self-insert-function 'priority 50) (defun electric-pair-will-use-region () (and (use-region-p) diff --git a/lisp/electric.el b/lisp/electric.el index ed968d7c65e..b9458776e3b 100644 --- a/lisp/electric.el +++ b/lisp/electric.el @@ -270,28 +270,29 @@ or comment." ;; hence copied). (let ((at-newline (<= pos (line-beginning-position)))) (when at-newline - (let ((before (copy-marker (1- pos) t))) + (let ((before (copy-marker (1- pos) t)) + inhibit-reindentation) (save-excursion - (unless (or (memq indent-line-function - electric-indent-functions-without-reindent) - electric-indent-inhibit) + (unless + (setq inhibit-reindentation + (or (memq indent-line-function + electric-indent-functions-without-reindent) + electric-indent-inhibit)) ;; Don't reindent the previous line if the ;; indentation function is not a real one. (goto-char before) (condition-case-unless-debug () (indent-according-to-mode) - (error (throw 'indent-error nil)))) - ;; We are at EOL before the call to - ;; `indent-according-to-mode', and after it we usually - ;; are as well, but not always. We tried to address - ;; it with `save-excursion' but that uses a normal - ;; marker whereas we need `move after insertion', so - ;; we do the save/restore by hand. - (goto-char before) - (when (eolp) - ;; Remove the trailing whitespace after indentation because - ;; indentation may (re)introduce the whitespace. - (delete-horizontal-space t))))) + (error (throw 'indent-error nil))) + ;; The goal here will be to remove the trailing + ;; whitespace after reindentation of the previous line + ;; because that may have (re)introduced it. + (goto-char before) + ;; We were at EOL in marker `before' before the call + ;; to `indent-according-to-mode' but after we may + ;; not be (Bug#15767). + (when (and (eolp)) + (delete-horizontal-space t)))))) (unless (and electric-indent-inhibit (not at-newline)) (condition-case-unless-debug () @@ -388,6 +389,10 @@ WHERE if the rule matches, or nil if it doesn't match. If multiple rules match, only first one is executed.") +;; TODO: Make this a defcustom? +(defvar electric-layout-allow-duplicate-newlines nil + "If non-nil, allow duplication of `before' newlines.") + (defun electric-layout-post-self-insert-function () (when electric-layout-mode (electric-layout-post-self-insert-function-1))) @@ -420,11 +425,14 @@ If multiple rules match, only first one is executed.") (lambda () ;; FIXME: we use `newline', which calls ;; `self-insert-command' and ran - ;; `post-self-insert-hook' recursively. It - ;; happened to make `electric-indent-mode' work - ;; automatically with `electric-layout-mode' (at - ;; the cost of re-indenting lines multiple times), - ;; but I'm not sure it's what we want. + ;; `post-self-insert-hook' recursively. It happened + ;; to make `electric-indent-mode' work automatically + ;; with `electric-layout-mode' (at the cost of + ;; re-indenting lines multiple times), but I'm not + ;; sure it's what we want. + ;; + ;; JT@19/02/22: Indeed in the case of `before' + ;; newlines, re-indentation is prevented. ;; ;; FIXME: when `newline'ing, we exceptionally ;; prevent a specific behaviour of @@ -438,10 +446,28 @@ If multiple rules match, only first one is executed.") (let ((electric-layout-mode nil) (electric-pair-open-newline-between-pairs nil)) (newline 1 t)))) - (nl-before (lambda () - (save-excursion - (goto-char (1- pos)) (skip-chars-backward " \t") - (unless (bolp) (funcall nl-after)))))) + (nl-before + (lambda () + (save-excursion + (goto-char (1- pos)) + ;; Normally, we don't duplicate newlines, but when + ;; we're being called for i.e. a closer brace for + ;; `electric-pair-mode' generally make sense. So + ;; consult `electric-layout-allow-duplicate-newlines' + (unless (and (not electric-layout-allow-duplicate-newlines) + (progn (skip-chars-backward " \t") + (bolp))) + ;; FIXME: JT@19/03/22: Make sure the `before' + ;; newline being inserted here does not trigger + ;; reindentation. It doesn't seem to be our job + ;; to do so and it break with `cc-mode's + ;; indentation function. Later on we can add a + ;; before-and-maybe-indent, or if the user + ;; really wants to reindent, then + ;; `last-command-event' should be in + ;; `electric-indent-chars'. + (let ((electric-indent-inhibit t)) + (funcall nl-after))))))) (pcase sym ('before (funcall nl-before)) ('after (funcall nl-after)) diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el index 0b076e4be01..4f1e5729be1 100644 --- a/test/lisp/electric-tests.el +++ b/test/lisp/electric-tests.el @@ -391,11 +391,12 @@ baz\"\"" :bindings '((electric-pair-skip-whitespace . chomp)) :test-in-comments nil) -(define-electric-pair-test whitespace-chomping-2 - " ( \n\t\t\n ) " "--)------" :expected-string " () " :expected-point 4 - :bindings '((electric-pair-skip-whitespace . chomp)) - :modes '(c++-mode) - :test-in-comments nil) +(ert-deftest electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings nil + "Check if whitespace chomping works in `c++' unterminated strings." + (electric-pair-test-for + "\" ( \n \n ) \"" 4 41 "\" () \"" 5 'c++-mode + '((electric-pair-skip-whitespace . chomp)) + (lambda () (electric-pair-mode 1)))) ;; A test failure introduced by: ;; ;; bb591f139f: Enhance CC Mode's fontification, etc., of unterminated strings. @@ -513,6 +514,7 @@ baz\"\"" :fixture-fn #'(lambda () (electric-pair-mode 1))) + (define-electric-pair-test js-mode-braces-with-layout "" "{" :expected-string "{\n\n}" :expected-point 3 :modes '(js-mode) @@ -532,6 +534,16 @@ baz\"\"" (electric-indent-mode 1) (electric-layout-mode 1))) +(define-electric-pair-test js-mode-braces-with-layout-and-indent + "" "{" :expected-string "{\n \n}" :expected-point 7 + :modes '(js-mode) + :test-in-comments nil + :test-in-strings nil + :fixture-fn #'(lambda () + (electric-pair-mode 1) + (electric-indent-mode 1) + (electric-layout-mode 1))) + ;;; Backspacing ;;; TODO: better tests @@ -821,6 +833,35 @@ baz\"\"" ;;; tests for `electric-layout-mode' +(define-derived-mode plainer-c-mode c-mode "pC" + "A plainer/saner C-mode with no internal electric machinery." + (c-toggle-electric-state -1) + (setq-local electric-indent-local-mode-hook nil) + (setq-local electric-indent-mode-hook nil) + (electric-indent-local-mode 1) + (dolist (key '(?\" ?\' ?\{ ?\} ?\( ?\) ?\[ ?\])) + (local-set-key (vector key) 'self-insert-command))) + +(defun electric-layout-for-c-style-du-jour (inserted) + "A function to use in `electric-layout-rules'" + (when (memq inserted '(?{ ?})) + (save-excursion + (backward-char 2) (c-point-syntax) (forward-char) ; silly, but needed + (c-brace-newlines (c-point-syntax))))) + +(ert-deftest electric-layout-plainer-c-mode-use-c-style () + (ert-with-test-buffer () + (plainer-c-mode) + (electric-layout-local-mode 1) + (electric-pair-local-mode 1) + (electric-indent-local-mode 1) + (setq-local electric-layout-rules + '(electric-layout-for-c-style-du-jour)) + (insert "int main () ") + (let ((last-command-event ?\{)) + (call-interactively (key-binding `[,last-command-event]))) + (should (equal (buffer-string) "int main ()\n{\n \n}\n")))) + (ert-deftest electric-layout-int-main-kernel-style () (ert-with-test-buffer () (plainer-c-mode) @@ -828,7 +869,8 @@ baz\"\"" (electric-pair-local-mode 1) (electric-indent-local-mode 1) (setq-local electric-layout-rules - '((?\{ . (after-stay after)))) + '((?\{ . (after)) + (?\} . (before)))) (insert "int main () ") (let ((last-command-event ?\{)) (call-interactively (key-binding `[,last-command-event]))) @@ -850,7 +892,8 @@ baz\"\"" (electric-pair-local-mode 1) (electric-indent-local-mode 1) (setq-local electric-layout-rules - '((?\{ . (before after-stay after)))) + '((?\{ . (before after)) + (?\} . (before)))) (insert "int main () ") (let ((last-command-event ?\{)) (call-interactively (key-binding `[,last-command-event]))) -- 2.39.2