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 ()
   {
     <indented point>
   }

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 () <newline>
  {
  <newline>

The e-p-m inserts the pair

  function () <newline>
  {
  <newline>}

And then e-l-m kicks in for the pair again, yielding the desired result

  function () <newline>
  {
  <indented point>
  }

* 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.

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])))