Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes non-whitespace

Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes non-whitespace

[Juanma Barranquero]

[-- Attachment #1.1: Type: text/plain, Size: 177 bytes --]

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?
​

[-- Attachment #1.2: Type: text/html, Size: 234 bytes --]

[-- Attachment #2: bug-21766.patch --]
[-- Type: application/octet-stream, Size: 2277 bytes --]

diff --git c/lisp/simple.el i/lisp/simple.el
index 338a060..f6c580f 100644
--- c/lisp/simple.el
+++ i/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 c/test/automated/data/simple/bug-21766.py i/test/automated/data/simple/bug-21766.py
new file mode 100644
index 0000000..118719a
--- /dev/null
+++ i/test/automated/data/simple/bug-21766.py
@@ -0,0 +1,4 @@
+query = """WITH filtered AS 
+WHERE      
+""".format(fv_)
+
diff --git c/test/automated/simple-test.el i/test/automated/simple-test.el
index 8da575d..62fe17c 100644
--- c/test/automated/simple-test.el
+++ i/test/automated/simple-test.el
@@ -21,6 +21,10 @@
 
 (require 'ert)
 
+(defvar simple-tests-data-directory
+  (expand-file-name "data/simple" (getenv "EMACS_TEST_DIRECTORY"))
+  "Directory containing simple.el test data.")
+
 (defmacro simple-test--dummy-buffer (&rest body)
   (declare (indent 0)
            (debug t))
@@ -180,5 +184,23 @@ simple-test--dummy-buffer
           (should (= x 2)))
       (remove-hook 'post-self-insert-hook inc))))
 
+\f
+;;; `delete-trailing-whitespace'
+(ert-deftest simple-delete-trailing-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-file-contents (expand-file-name "bug-21766.py"
+                                                  simple-tests-data-directory))
+          (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

Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes non-whitespace

[Stephen Leake]

Juanma Barranquero <lekktu@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

Re: bug#21766: 25.0.50; delete-trailing-whitespace sometimes deletes non-whitespace

[Juanma Barranquero]

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

On Wed, Oct 28, 2015 at 2:51 AM, Stephen Leake <
stephen_leake@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

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