bug#35254: 27.0.50; cc-mode/electric-pair-mode/electric-layout-mode: bad trailing whitespace behavior in cc-mode

["João Távora" <joaotavora@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5817 bytes --]

Well,

I've just come across the bug myself, and it is indeed annoying.
Can you check if this patch, which seems the simplest, serves
all purposes? It also adds a test to prevent future regressions

Thanks,
João

diff --git a/lisp/electric.el b/lisp/electric.el
index 657913a396..f15a95bf91 100644
--- a/lisp/electric.el
+++ b/lisp/electric.el
@@ -281,10 +281,13 @@ electric-indent-post-self-insert-function
                   (goto-char before)
                   (condition-case-unless-debug ()
                       (indent-according-to-mode)
-                    (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.
+                    (error (throw 'indent-error nil))))
+                (unless (eq electric-indent-inhibit 'electric-layout-mode)
+                  ;; Unless we're operating under
+                  ;; `electric-layout-mode' (Bug#35254), 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
@@ -464,7 +467,7 @@ electric-layout-post-self-insert-function-1
                       ;; really wants to reindent, then
                       ;; `last-command-event' should be in
                       ;; `electric-indent-chars'.
-                      (let ((electric-indent-inhibit t))
+                      (let ((electric-indent-inhibit
'electric-layout-mode))
                         (funcall nl-after)))))))
             (pcase sym
               ('before (funcall nl-before))
diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 4f1e5729be..22b7a18ba5 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -876,6 +876,24 @@ electric-layout-for-c-style-du-jour
       (call-interactively (key-binding `[,last-command-event])))
     (should (equal (buffer-string) "int main () {\n  \n}"))))

+(ert-deftest electric-layout-control-reindentation ()
+  "Same as `e-l-int-main-kernel-style', but checking Bug#35254."
+  (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
+                '((?\{ . (after))
+                  (?\} . (before))))
+    (insert "int main () ")
+    (let ((last-command-event ?\{))
+      (call-interactively (key-binding `[,last-command-event])))
+    (should (equal (buffer-string) "int main () {\n  \n}"))
+    ;; insert an additional newline and check indentation
+    (call-interactively 'newline)
+    (should (equal (buffer-string) "int main () {\n\n  \n}"))))
+
 (define-derived-mode plainer-c-mode c-mode "pC"
   "A plainer/saner C-mode with no internal electric machinery."
   (c-toggle-electric-state -1)




On Wed, May 15, 2019 at 11:03 AM Alan Mackenzie <acm@muc.de> wrote:

> Hello, João.
>
> On Tue, May 14, 2019 at 11:34:24 +0100, João Távora wrote:
> > On Tue, May 14, 2019 at 10:27 AM Alan Mackenzie <acm@muc.de> wrote:
>
> > > The bug is, type lots of <CR>s in a row; the indentation WS isn't
> > > getting removed from the blank lines.  Currently
> electric-indent-inhibit
> > > is inhibiting this removal.
>
>
> > Do you mean the "removal of the WS in the lines preceding the current".
> > In other words, do you mean "removal of the trailing WS that was once
> > proper indentation"?
>
> Yes.  To be absolutely clear, supposing we have point at the end of a
> line containing nothing but indentation space (e.g., we've just typed
> <CR>):
>
> <spaces>!
>         ^
>       point
>
> Type <CR> again.  What we are currently seeing is:
>
> <spaces>
> <spaces>!
>
> .  What we want to see is
>
> <nothing>
> <spaces>!
>
> .
>
> > Or do you think that the current line, the one where point stands, should
> > not be indented at all in certain electric-* variable combinations and or
> > c-electric-* variable?  Which of those combinations?
>
> When electric-indent-inhibit is set, the (electric) indentation of the
> current line should not be done by electric-indent-mode.  For the
> moment, in CC Mode it should be done by c-electric-brace, and friends,
> if so configured in CC Mode (the default being enabled).
>
> > > Probably.  Maybe João should check this, once he's fully back with us.
>
>
> > I'm afraid I can't put a date on that. There's a bun in the oven...
>
> Well, congratulations!  I hope everything goes well.
>
> > An important development towards figuring out this issue is that a
> > significant fraction of us agrees on what the behavior should be
> > in what cases.  Then we should code tests that assert that behavior
> > possibly reusing the fixtures in electric-tests.el.
>
> Yes.
>
> > > The same bug occurs in Python Mode.
> > > Succinctly, the bug is that on pressing <CR> lots of times in a row,
> the
> > > indentation WS is being left on the blank lines rather than being
> > > removed.
>
> > I see.  That does make sense. But, to be sure, we _dont_ what to
> > remove the indentation WS on the "current" line, right?
>
> Right.  Unless, and until, the current line becomes the "previous" line,
> still otherwise being blank.
>
> I think we're agreed on everything.  :-)
>
> > João
>
> --
> Alan Mackenzie (Nuremberg, Germany).
>


-- 
João Távora

[-- Attachment #2: Type: text/html, Size: 7565 bytes --]