unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Alan Mackenzie <acm@muc.de>
To: "João Távora" <joaotavora@gmail.com>
Cc: Noam Postavsky <npostavs@gmail.com>,
	Stefan Monnier <monnier@iro.umontreal.ca>,
	35254@debbugs.gnu.org, Dima Kogan <dima@secretsauce.net>
Subject: bug#35254: 27.0.50; cc-mode/electric-pair-mode/electric-layout-mode: bad trailing whitespace behavior in cc-mode
Date: Tue, 14 May 2019 08:38:55 +0000	[thread overview]
Message-ID: <20190514083855.GA4231__16798.2748987683$1557823191$gmane$org@ACM> (raw)
In-Reply-To: <CALDnm50hwdwXd6_e23OdN8NRtgfRjXNNq7Q=VR4W4us=J+BJ5w@mail.gmail.com>

Hello, João.

On Mon, May 13, 2019 at 23:39:33 +0100, João Távora wrote:
> On Mon, May 13, 2019 at 8:53 PM Alan Mackenzie <acm@muc.de> wrote:

[ .... ]

> > > Electric indent mode's post-self-insert hook entry has 3 effects:

> > > 1. Indent the previous line.
> > > 2. Remove trailing whitespace from the previous line.
> > > 3. Indent the current line (when at beginning of line).

> > > The change from 2019-01-22 "electric-layout-mode kicks in before
> > > electric-pair-mode", makes 'electric-indent-inhibit' inhibit 1 and 2,
> > > whereas before then it inhibited only 1.  While cc mode provides its
> > > own electric commands and therefore sets 'electric-indent-inhibit', it
> > > doesn't implement an electric newline command.  So if only one of
> > > effects 2 and 3 from Electric indent mode occur, then hitting RET will
> > > leave trailing whitespace.

> > I interpret the problem a little differently.
> > electric-indent-post-self-insert-function, when electric-indent-inhibit
> > is set, is inhibiting actions which are not really part of electric
> > indentation, in particular action 2 (above).  This is the heart of the
> > bug.  The following patch fixes the bug.  It would need tidying up before
> > being committed:



> > diff --git a/lisp/electric.el b/lisp/electric.el
> > index 07da2f1d9e..15a42930c1 100644
> > --- a/lisp/electric.el
> > +++ b/lisp/electric.el
> > @@ -282,9 +282,15 @@ electric-indent-post-self-insert-function
> >                    (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.
> > +                  )
> > +                (unless (memq indent-line-function
> > +                              electric-indent-functions-without-reindent)
> > +                  ;; The goal here will be to remove the indentation
> > +                  ;; whitespace from an otherwise blank line after
> > +                  ;; typing <CR> twice in succession.  Also to remove
> > +                  ;; 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


> > João and Noam, what're your views on this proposed patch?

> > Good night Alan,

> Unfortunately, I can't analyse this in depth right now or in the next
> few days.

As Stefan(?) said, there's no particular hurry (on a scale of days) to
fix this.

> I can only ask succintly:

> 1. Does it fix the reported problem (assuming it is a problem, and not an
> otherwise potentially desirable change in behaviour)?

It does fix it, yes.  It was a bug.

> 2. Do any of you have suspicions that it might introduce problems
> elsewhere?

I don't, as such, but I haven't dug around the code looking for potential
problems.  That was the main reason I was asking you, as maintainer of
the electric stuff.

> 3. Does it pass the automated test suite?

Noam has already tried this and said it fails on three tests.

> I am only a bit wary of it because, without having understood the
> problem in depth, it seems again caused by a c-electric quirk.

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.

> My views on this are known: I always prefer that c-mode would, if
> slowly, move in the direction of accepting electric.el abstractions as
> closely as possible.

I think we've already agreed that CC Mode should, in the long term,
change to using the electric-* facilities, assuming no loss in
functionality.  :-)

> Than said, the patch looks very simple, which is always good, and has
> comments explaining what's going on. So I'll let Noam (and Stefan?) decide.

> João

-- 
Alan Mackenzie (Nuremberg, Germany).





  parent reply	other threads:[~2019-05-14  8:38 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-13  6:32 bug#35254: 27.0.50; cc-mode/electric-pair-mode/electric-layout-mode: bad trailing whitespace behavior in cc-mode Dima Kogan
2019-05-11  3:12 ` Noam Postavsky
2019-05-11 12:05   ` Alan Mackenzie
     [not found]   ` <20190511120524.GA15991@ACM>
2019-05-11 14:06     ` Noam Postavsky
2019-05-11 16:19       ` Alan Mackenzie
2019-05-11 19:34         ` Basil L. Contovounesios
2019-05-12 16:14           ` Alan Mackenzie
2019-05-12 21:45             ` Basil L. Contovounesios
2019-05-13 10:14               ` Alan Mackenzie
     [not found]               ` <20190513101448.GA5525@ACM>
2019-05-13 12:49                 ` Basil L. Contovounesios
2019-05-12 15:12         ` Alan Mackenzie
2019-05-12 18:42           ` Noam Postavsky
2019-05-13 19:53   ` Alan Mackenzie
     [not found]   ` <20190513195323.GB5525@ACM>
2019-05-13 22:39     ` João Távora
2019-05-13 23:38       ` Noam Postavsky
2019-05-14  1:20         ` João Távora
2019-05-14  1:28         ` Stefan Monnier
2019-05-14  1:56       ` Noam Postavsky
2019-05-14  8:38       ` Alan Mackenzie [this message]
2019-05-13 23:32     ` Stefan Monnier
     [not found]     ` <jwvimue9bzj.fsf-monnier+emacs@gnu.org>
2019-05-13 23:45       ` Noam Postavsky
2019-05-14  1:26         ` Stefan Monnier
2019-05-14  9:27       ` Alan Mackenzie
2019-05-14  9:34       ` Alan Mackenzie
     [not found]       ` <20190514092735.GB4231@ACM>
2019-05-14 10:34         ` João Távora
2019-05-15 10:03           ` Alan Mackenzie
2019-05-15 11:27             ` João Távora
2019-05-15 13:19               ` Stefan Monnier
2019-05-15 13:55                 ` João Távora
2019-05-15 14:03                   ` João Távora
2019-07-01 12:24                   ` João Távora
2019-07-01 13:34                     ` Alan Mackenzie
     [not found]                     ` <20190701133427.GA23312@ACM>
2019-07-06 16:24                       ` Noam Postavsky
2019-07-06 22:24                         ` João Távora
2019-07-06 22:50                           ` Noam Postavsky
2019-07-06 22:33                       ` João Távora
     [not found]       ` <20190514093415.GC4231@ACM>
2019-05-14 15:38         ` Stefan Monnier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='20190514083855.GA4231__16798.2748987683$1557823191$gmane$org@ACM' \
    --to=acm@muc.de \
    --cc=35254@debbugs.gnu.org \
    --cc=dima@secretsauce.net \
    --cc=joaotavora@gmail.com \
    --cc=monnier@iro.umontreal.ca \
    --cc=npostavs@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).