GNU bug report logs - #21766
25.0.50; delete-trailing-whitespace sometimes deletes non-whitespace

Previous Next

Package: emacs;

Reported by: Markus Triska <triska <at> metalevel.at>

Date: Mon, 26 Oct 2015 23:10:01 UTC

Severity: normal

Merged with 21769

Found in version 25.0.50

Done: Juanma Barranquero <lekktu <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 21766 in the body.
You can then email your comments to 21766 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Mon, 26 Oct 2015 23:10:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Markus Triska <triska <at> metalevel.at>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 26 Oct 2015 23:10:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Markus Triska <triska <at> metalevel.at>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.50; delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 00:07:10 +0100
Steps to reproduce:

1) wget http://www.metalevel.at/ei/fault1.html

2) emacs -Q fault1.html

3) M-x delete-trailing-whitespace RET

This removes several non-white-space characters, including the whole
line containing "and use it like" (line 83 in the original file),
yielding the following diff against the original file:

   http://www.metalevel.at/ei/fault1.patch

Subtle changes in different sections of the file cause different
behaviour of `delete-trailing-whitespace'. For example, if I remove the
initial paragraph (patch: http://www.metalevel.at/ei/fault12.patch) to
obtain the slightly altered file:

   http://www.metalevel.at/ei/fault2.html

Then the analogous steps, using fault2.html ($ emacs -Q fault2.html,
M-x delete-trailing-whitespace RET) work as expected, removing only
trailing whitespace characters and leading to the diff:

   http://www.metalevel.at/ei/fault2.patch

All steps work as expected in Emacs 24.5, removing only trailing
whitespace characters in all cases.

Thank you for looking into this!
Markus


In GNU Emacs 25.0.50.5 (x86_64-apple-darwin14.1.0, X toolkit, Xaw3d scroll bars)
 of 2015-10-04
Repository revision: acfb5cd0353406784f085ddb6edfb0d0587048c8
Windowing system distributor 'The X.Org Foundation', version 11.0.11502000
Configured using:
 'configure --without-ns CFLAGS=-I/opt/local/include
 LDFLAGS=-L/opt/local/lib'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG IMAGEMAGICK GSETTINGS NOTIFY ACL GNUTLS
LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS LUCID X11

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 00:17:02 GMT) Full text and rfc822 format available.

Message #8 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Markus Triska <triska <at> metalevel.at>
Cc: 21766 <at> debbugs.gnu.org
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 01:15:18 +0100
[Message part 1 (text/plain, inline)]
On Tue, Oct 27, 2015 at 12:07 AM, Markus Triska <triska <at> metalevel.at> wrote:
>
>
> Steps to reproduce:
>
> 1) wget http://www.metalevel.at/ei/fault1.html
>
> 2) emacs -Q fault1.html
>
> 3) M-x delete-trailing-whitespace RET
>
> This removes several non-white-space characters, including the whole
> line containing "and use it like" (line 83 in the original file),

Apparently, the call to skip-syntax-backward is altering the match data. If
you try this

--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -609,7 +609,7 @@ delete-trailing-whitespace
             (start (or start (point-min))))
         (goto-char start)
         (while (re-search-forward "\\s-$" end-marker t)
-          (skip-syntax-backward "-" (line-beginning-position))
+          (save-match-data (skip-syntax-backward "-"
(line-beginning-position)))
           ;; Don't delete formfeeds, even if they are considered
whitespace.
           (if (looking-at-p ".*\f")
               (goto-char (match-end 0)))


or, alternatively,

@@ -609,11 +609,12 @@ delete-trailing-whitespace
             (start (or start (point-min))))
         (goto-char start)
         (while (re-search-forward "\\s-$" end-marker t)
-          (skip-syntax-backward "-" (line-beginning-position))
-          ;; Don't delete formfeeds, even if they are considered
whitespace.
-          (if (looking-at-p ".*\f")
-              (goto-char (match-end 0)))
-          (delete-region (point) (match-end 0)))
+          (let ((end (match-end 0)))
+            (skip-syntax-backward "-" (line-beginning-position))
+            ;; Don't delete formfeeds, even if they are considered
whitespace.
+            (if (looking-at-p ".*\f")
+                (goto-char end))
+            (delete-region (point) end)))
         ;; Delete trailing empty lines.
         (goto-char end-marker)
         (when (and (not end)

it works. Now, the question is, should skip-syntax-backward preserve the
match data, or must delete-trailing-whitespace be coded defensively? The
usual policy has been that code that needs the match data preserved should
make sure itself that it is so.
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 07:08:02 GMT) Full text and rfc822 format available.

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes
 non-whitespace
Date: Tue, 27 Oct 2015 08:07:38 +0100
[Message part 1 (text/plain, inline)]
 On 27.10.2015 01:15, Juanma Barranquero wrote:
> On Tue, Oct 27, 2015 at 12:07 AM, Markus Triska <triska <at> metalevel.at 
> <mailto:triska <at> metalevel.at>> wrote:
> >
> >
> > Steps to reproduce:
> >
> > 1) wget http://www.metalevel.at/ei/fault1.html
> >
> > 2) emacs -Q fault1.html
> >
> > 3) M-x delete-trailing-whitespace RET
> >
> > This removes several non-white-space characters, including the whole
> > line containing "and use it like" (line 83 in the original file),
>
> Apparently, the call to skip-syntax-backward is altering the match 
> data. If you try this
>
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -609,7 +609,7 @@ delete-trailing-whitespace
>              (start (or start (point-min))))
>          (goto-char start)
>          (while (re-search-forward "\\s-$" end-marker t)
> -          (skip-syntax-backward "-" (line-beginning-position))
> +          (save-match-data (skip-syntax-backward "-" 
> (line-beginning-position)))
>            ;; Don't delete formfeeds, even if they are considered 
> whitespace.
>            (if (looking-at-p ".*\f")
>                (goto-char (match-end 0)))
>
>
> or, alternatively,
>
> @@ -609,11 +609,12 @@ delete-trailing-whitespace
>              (start (or start (point-min))))
>          (goto-char start)
>          (while (re-search-forward "\\s-$" end-marker t)
> -          (skip-syntax-backward "-" (line-beginning-position))
> -          ;; Don't delete formfeeds, even if they are considered 
> whitespace.
> -          (if (looking-at-p ".*\f")
> -              (goto-char (match-end 0)))
> -          (delete-region (point) (match-end 0)))
> +          (let ((end (match-end 0)))
> +            (skip-syntax-backward "-" (line-beginning-position))
> +            ;; Don't delete formfeeds, even if they are considered 
> whitespace.
> +            (if (looking-at-p ".*\f")
> +                (goto-char end))
> +            (delete-region (point) end)))
>          ;; Delete trailing empty lines.
>          (goto-char end-marker)
>          (when (and (not end)
>
> it works.

First fix looks cleaner.

> Now, the question is, should skip-syntax-backward preserve the match 
> data, or must delete-trailing-whitespace be coded defensively? The 
> usual policy has been that code that needs the match data preserved 
> should make sure itself that it is so.

Such a basic routine should preserve matches by its own virtue.

A couple of regression tests should pay here.

Thanks
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 07:55:02 GMT) Full text and rfc822 format available.

Message #14 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 21766 <at> debbugs.gnu.org
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 08:53:28 +0100
[Message part 1 (text/plain, inline)]
On Tue, Oct 27, 2015 at 8:07 AM, Andreas Röhler <
andreas.roehler <at> easy-emacs.de> wrote:

> First fix looks cleaner.

Shorter, indeed, though it does more work, and inside a loop. I really have
no preference.

> Such a basic routine should preserve matches by its own virtue.

Are you talking about delete-trailing-whitespace or skip-syntax-backward?

Anyway, I've always thought that functions should preserve the match data,
but I was told that doing so is a (relatively) expensive operation, and
functions that use match data should be the ones to make sure that it is
valid when they need it.

    J
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 08:07:02 GMT) Full text and rfc822 format available.

Message #17 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juanma Barranquero <lekktu <at> gmail.com>, Markus Triska <triska <at> metalevel.at>
Cc: 21766 <at> debbugs.gnu.org
Subject: Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes
 non-whitespace
Date: Tue, 27 Oct 2015 09:05:52 +0100
> Now, the question is, should skip-syntax-backward preserve the
> match data, or must delete-trailing-whitespace be coded defensively?

The latter.

> The
> usual policy has been that code that needs the match data preserved should
> make sure itself that it is so.

Shouldn't we remove that "\\s-$" rigmarole?  Something like the largely
untested

      (let ((end-marker (copy-marker (or end (point-max))))
	    old-bol new-eol new-bol)
        (goto-char (or start (point-min)))
        (while (re-search-forward "\\\n" end-marker t)
	  (setq new-bol (point))
	  (goto-char (setq new-eol (match-beginning 0)))
          (if (or (zerop (skip-syntax-backward
			  "-" (or old-bol (line-beginning-position))))
		  (looking-at-p ".*\f"))
	      (goto-char new-bol)
	    (delete-region (point) new-eol)
	    (forward-char))
	  (setq old-bol (point)))

It's still not very clean (should line-beginning-position be allowed to
go before START?) so if someone wants to polish it up ....

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 08:14:02 GMT) Full text and rfc822 format available.

Message #20 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 09:12:23 +0100
[Message part 1 (text/plain, inline)]
On Tue, Oct 27, 2015 at 9:05 AM, martin rudalics <rudalics <at> gmx.at> wrote:

> The latter.

Yep, that's the policy.

> Shouldn't we remove that "\\s-quot; rigmarole?  Something like the largely
> untested

Hmm, what's the gain for the added complexity?

    J
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 08:26:01 GMT) Full text and rfc822 format available.

Message #23 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juanma Barranquero <lekktu <at> gmail.com>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes
 non-whitespace
Date: Tue, 27 Oct 2015 09:25:15 +0100
> Hmm, what's the gain for the added complexity?

Maybe none.  I have no idea how the regexp machine works when looking
for "\\s-$".  If it looks for whitespace followed by a line end and not
for whitespace preceding a line end, the old version will be slow with
indented code.

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 08:34:01 GMT) Full text and rfc822 format available.

Message #26 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 09:32:49 +0100
[Message part 1 (text/plain, inline)]
On Tue, Oct 27, 2015 at 9:25 AM, martin rudalics <rudalics <at> gmx.at> wrote:

> If it looks for whitespace followed by a line end and not
> for whitespace preceding a line end, the old version will be slow with
> indented code.

I don't think we've ever had a bug report / complaint about
delete-trailing-whitespace's performance. It's usually used either
interactively or as a hook before saving the file, and in both cases I'd
bet the time it takes is dwarfed by human reaction times or by the time
spent saving the file to disk.

    J
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 09:02:02 GMT) Full text and rfc822 format available.

Message #29 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> suse.de>
To: Juanma Barranquero <lekktu <at> gmail.com>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 10:01:48 +0100
Juanma Barranquero <lekktu <at> gmail.com> writes:

> Apparently, the call to skip-syntax-backward is altering the match
> data.

No, skip-syntax-backward is safe.  There must be something in
line-beginning-position that does this, but only indirectly.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 09:18:02 GMT) Full text and rfc822 format available.

Message #32 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Andreas Schwab <schwab <at> suse.de>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 10:16:28 +0100
[Message part 1 (text/plain, inline)]
On Tue, Oct 27, 2015 at 10:01 AM, Andreas Schwab <schwab <at> suse.de> wrote:

> No, skip-syntax-backward is safe.  There must be something in
> line-beginning-position that does this, but only indirectly.

  (skip-syntax-backward "-"
        (save-match-data
(line-beginning-position)))

still shows the bug, while

  (save-match-data
    (skip-syntax-backward "-" (line-beginning-position)))

fixes it.
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 09:25:02 GMT) Full text and rfc822 format available.

Message #35 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juanma Barranquero <lekktu <at> gmail.com>, 
 Andreas Schwab <schwab <at> suse.de>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes
 non-whitespace
Date: Tue, 27 Oct 2015 10:24:28 +0100
>> No, skip-syntax-backward is safe.  There must be something in
>> line-beginning-position that does this, but only indirectly.
>
>    (skip-syntax-backward "-"
>          (save-match-data
> (line-beginning-position)))
>
> still shows the bug, while
>
>    (save-match-data
>      (skip-syntax-backward "-" (line-beginning-position)))
>
> fixes it.

I doubt that ‘syntax-propertize’ is always safe.

martin





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 09:47:02 GMT) Full text and rfc822 format available.

Message #38 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> suse.de>
To: Juanma Barranquero <lekktu <at> gmail.com>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 10:46:15 +0100
Juanma Barranquero <lekktu <at> gmail.com> writes:

> On Tue, Oct 27, 2015 at 10:01 AM, Andreas Schwab <schwab <at> suse.de> wrote:
>
>> No, skip-syntax-backward is safe.  There must be something in
>> line-beginning-position that does this, but only indirectly.
>
>   (skip-syntax-backward "-"
>         (save-match-data
> (line-beginning-position)))
>
> still shows the bug, while
>
>   (save-match-data
>     (skip-syntax-backward "-" (line-beginning-position)))
>
> fixes it.

Ok, I see it now, it is the new parse_sexp_propertize stuff which is
called by SETUP_SYNTAX_TABLE.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 10:23:01 GMT) Full text and rfc822 format available.

Message #41 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Andreas Schwab <schwab <at> suse.de>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 11:21:14 +0100
[Message part 1 (text/plain, inline)]
On Tue, Oct 27, 2015 at 10:46 AM, Andreas Schwab <schwab <at> suse.de> wrote:

> Ok, I see it now, it is the new parse_sexp_propertize stuff which is
> called by SETUP_SYNTAX_TABLE.

Then, can I assume this is not a bug in skip-syntax-backward or
parse_sexp_propetize, and fix delete-trailing-whitespace?
[Message part 2 (text/html, inline)]

Merged 21766 21769. Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 27 Oct 2015 14:02:02 GMT) Full text and rfc822 format available.

Added indication that bug 21766 blocks19759 Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 27 Oct 2015 14:02:03 GMT) Full text and rfc822 format available.

Merged 21766 21769. Request was from Juanma Barranquero <lekktu <at> gmail.com> to control <at> debbugs.gnu.org. (Tue, 27 Oct 2015 14:02:04 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Tue, 27 Oct 2015 16:05:01 GMT) Full text and rfc822 format available.

Message #50 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Andreas Schwab <schwab <at> suse.de>
Cc: 21766 <at> debbugs.gnu.org, Markus Triska <triska <at> metalevel.at>,
 Emacs developers <emacs-devel <at> gnu.org>
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 17:03:20 +0100
[Message part 1 (text/plain, inline)]
This will be my first ERT test, so, does the following (attached, because
Gmail) patch look acceptable?

And, it is possible to commit a file with trailing whitespace?
​
[Message part 2 (text/html, inline)]
[bug-21766.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Wed, 28 Oct 2015 01:53:02 GMT) Full text and rfc822 format available.

Message #53 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Leake <stephen_leake <at> stephe-leake.org>
To: Juanma Barranquero <lekktu <at> gmail.com>
Cc: Andreas Schwab <schwab <at> suse.de>, Emacs developers <emacs-devel <at> gnu.org>,
 Markus Triska <triska <at> metalevel.at>, 21766 <at> debbugs.gnu.org
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Tue, 27 Oct 2015 20:51:42 -0500
Juanma Barranquero <lekktu <at> gmail.com> writes:

> This will be my first ERT test, so, does the following (attached, because
> Gmail) patch look acceptable?

Looks good to me, except see below.

> And, it is possible to commit a file with trailing whitespace?

Yes, but it's also very easy to lose it, thus breaking the test. I
suggest instead to save the file without extra whitespace, and add code
to the test to insert the extra whitespace. That way you can also add
comments about why this test exists, etc.

-- 
-- Stephe




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Wed, 28 Oct 2015 09:10:02 GMT) Full text and rfc822 format available.

Message #56 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Stephen Leake <stephen_leake <at> stephe-leake.org>
Cc: Andreas Schwab <schwab <at> suse.de>, Emacs developers <emacs-devel <at> gnu.org>,
 Markus Triska <triska <at> metalevel.at>, 21766 <at> debbugs.gnu.org
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Wed, 28 Oct 2015 10:08:14 +0100
[Message part 1 (text/plain, inline)]
On Wed, Oct 28, 2015 at 2:51 AM, Stephen Leake <
stephen_leake <at> stephe-leake.org> wrote:

> I suggest instead to save the file without extra whitespace, and add code
> to the test to insert the extra whitespace.

The file is quite short, so in this case is better to get rid of it and
just insert its contents into the buffer.

Thanks,

     Juanma



        Fix bug#21766 and add test.

        * lisp/simple.el (delete-trailing-whitespace): Save match data when
        calling `skip-syntax-backward'.
        * test/automated/simple-test.el (simple-delete-trailing-whitespace):
        New test.


diff --git a/lisp/simple.el b/lisp/simple.el
index 338a060..f6c580f 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -609,7 +609,8 @@ delete-trailing-whitespace
             (start (or start (point-min))))
         (goto-char start)
         (while (re-search-forward "\\s-$" end-marker t)
-          (skip-syntax-backward "-" (line-beginning-position))
+          (save-match-data
+            (skip-syntax-backward "-" (line-beginning-position)))
           ;; Don't delete formfeeds, even if they are considered
whitespace.
           (if (looking-at-p ".*\f")
               (goto-char (match-end 0)))
diff --git a/test/automated/simple-test.el b/test/automated/simple-test.el
index 8da575d..5bfb746 100644
--- a/test/automated/simple-test.el
+++ b/test/automated/simple-test.el
@@ -180,5 +180,27 @@ simple-test--dummy-buffer
           (should (= x 2)))
       (remove-hook 'post-self-insert-hook inc))))

+
+;;; `delete-trailing-whitespace'
+(ert-deftest simple-delete-trailing-whitespace ()
+  "Test bug#21766: delete-whitespace sometimes deletes non-whitespace."
+  (defvar python-indent-guess-indent-offset)  ; to avoid a warning
+  (let ((python (featurep 'python))
+        (python-indent-guess-indent-offset nil)
+        (delete-trailing-lines t))
+    (unwind-protect
+        (with-temp-buffer
+          (python-mode)
+          (insert (concat "query = \"\"\"WITH filtered AS \n"
+                          "WHERE      \n"
+                          "\"\"\".format(fv_)\n"
+                          "\n"
+                          "\n"))
+          (delete-trailing-whitespace)
+          (should (equal (count-lines (point-min) (point-max)) 3)))
+      ;; Let's clean up if running interactive
+      (unless (or noninteractive python)
+        (unload-feature 'python)))))
+
 (provide 'simple-test)
 ;;; simple-test.el ends here
-- 
2.6.2.windows.1
[Message part 2 (text/html, inline)]

Reply sent to Juanma Barranquero <lekktu <at> gmail.com>:
You have taken responsibility. (Wed, 28 Oct 2015 17:39:01 GMT) Full text and rfc822 format available.

Notification sent to Markus Triska <triska <at> metalevel.at>:
bug acknowledged by developer. (Wed, 28 Oct 2015 17:39:02 GMT) Full text and rfc822 format available.

Message #61 received at 21766-done <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Stephen Leake <stephen_leake <at> stephe-leake.org>
Cc: Andreas Schwab <schwab <at> suse.de>, 21766-done <at> debbugs.gnu.org,
 Markus Triska <triska <at> metalevel.at>
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Wed, 28 Oct 2015 18:38:07 +0100
[Message part 1 (text/plain, inline)]
Fixed in commit 1f02cbea8b489ed7676110431aa36ad5abc47d9b
​
[Message part 2 (text/html, inline)]

Reply sent to Juanma Barranquero <lekktu <at> gmail.com>:
You have taken responsibility. (Wed, 28 Oct 2015 17:39:02 GMT) Full text and rfc822 format available.

Notification sent to Puneeth Chaganti <punchagan <at> muse-amuse.in>:
bug acknowledged by developer. (Wed, 28 Oct 2015 17:39:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Wed, 28 Oct 2015 19:20:03 GMT) Full text and rfc822 format available.

Message #69 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Andreas Schwab <schwab <at> suse.de>
Cc: Juanma Barranquero <lekktu <at> gmail.com>, Markus Triska <triska <at> metalevel.at>,
 21766 <at> debbugs.gnu.org
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Wed, 28 Oct 2015 15:19:58 -0400
>> (save-match-data
>> (skip-syntax-backward "-" (line-beginning-position)))
[...]
> Ok, I see it now, it is the new parse_sexp_propertize stuff which is
> called by SETUP_SYNTAX_TABLE.

Indeed, internal--syntax-propertize needs to be fixed so it preserves
the match-data!


        Stefan






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Thu, 29 Oct 2015 06:31:02 GMT) Full text and rfc822 format available.

Message #72 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes
 non-whitespace
Date: Thu, 29 Oct 2015 07:30:15 +0100
 On 28.10.2015 20:19, Stefan Monnier wrote:
>>> (save-match-data
>>> (skip-syntax-backward "-" (line-beginning-position)))
> [...]
>> Ok, I see it now, it is the new parse_sexp_propertize stuff which is
>> called by SETUP_SYNTAX_TABLE.
> Indeed, internal--syntax-propertize needs to be fixed so it preserves
> the match-data!
>
>
>          Stefan
>
>
>
>
>

When internal--syntax-propertize is fixed, the current fix will be 
useless, i.e. redundant.

Is there a task-keeper for this scenario?






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#21766; Package emacs. (Thu, 29 Oct 2015 09:43:01 GMT) Full text and rfc822 format available.

Message #75 received at 21766 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 21766 <at> debbugs.gnu.org
Subject: Re: bug#21766: 25.0.50;
 delete-trailing-whitespace sometimes deletes non-whitespace
Date: Thu, 29 Oct 2015 10:41:12 +0100
[Message part 1 (text/plain, inline)]
On Thu, Oct 29, 2015 at 7:30 AM, Andreas Röhler <
andreas.roehler <at> easy-emacs.de> wrote:

> When internal--syntax-propertize is fixed, the current fix will be
useless, i.e. redundant.

Stefan already reverted the fix to delete-trailing-whitespace and fixed
internal--syntax-propertize in d7a67c5a2fe63b6f087d6cae24c8f3b3c09eb57a
[Message part 2 (text/html, inline)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 26 Nov 2015 12:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 9 years and 210 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.