Commit fd943124 authored by João Távora's avatar João Távora

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>

The e-p-m inserts the pair

  function () <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-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

* 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-layout-int-main-kernel-style): Simplify
(electric-layout-for-c-style-du-jour): New helper.
(electric-layout-plainer-c-mode-use-c-style): New test.
parent 6ca4626c
Pipeline #593 failed with stage
in 41 minutes and 10 seconds
......@@ -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
......@@ -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)
(goto-char pos)
(funcall electric-pair-skip-self last-command-event))
(when (and (not (and unconditional
......@@ -525,8 +527,10 @@ happened."
((and (memq syntax '(?\( ?\" ?\$))
(not overwrite-mode)
(or unconditional
(not (funcall electric-pair-inhibit-predicate
(not (save-excursion
(goto-char pos)
(funcall electric-pair-inhibit-predicate
(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)
......@@ -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))
(unless (or (memq indent-line-function
(setq inhibit-reindentation
(or (memq indent-line-function
;; Don't reindent the previous line if the
;; indentation function is not a real one.
(goto-char before)
(condition-case-unless-debug ()
(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
......@@ -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 ()
(goto-char (1- pos)) (skip-chars-backward " \t")
(unless (bolp) (funcall nl-after))))))
(lambda ()
(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")
;; 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))
......@@ -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."
"\" ( \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 '(?{ ?}))
(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 ()
(electric-layout-local-mode 1)
(electric-pair-local-mode 1)
(electric-indent-local-mode 1)
(setq-local electric-layout-rules
(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 ()
......@@ -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])))
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment