bug#23843: [PATCH 0/3] Small `delete-trailing-whitespace' fixes and improvements

bug#23843: [PATCH 0/3] Small `delete-trailing-whitespace' fixes and improvements

[Michal Nazarewicz]

Some small fixes and improvements to the ‘delete-trailing-whitespace’
command.  Unless there are objections, I’ll push it in a week or so.

Michal Nazarewicz (3):
  Make ‘delete-trailing-whitespace’ delete spaces after form feed
  Simplify ‘delete-trailing-whitespace’ by not treating \n as whitespace
  Don’t create unnecessary marker in ‘delete-trailing-whitespace’

 etc/NEWS                  |  6 ++++++
 lisp/simple.el            | 33 ++++++++++++++++-----------------
 test/lisp/simple-tests.el | 18 ++++++++++++++++--
 3 files changed, 38 insertions(+), 19 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

bug#23843: [PATCH 1/3] Make ‘delete-trailing-whitespace’ delete spaces after form feed

[Michal Nazarewicz]

* lisp/simple.el (delete-trailing-whitespace): Treat form fead as
a non-whitespace character (regradless of whether it’s character syntax
is whitespace) and delete any whitespace following it instead of leaving
lines with form feeds completely unchanged.  I.e. a line like "\f " will
now became "\f".
---
 etc/NEWS                  |  6 ++++++
 lisp/simple.el            | 15 +++++++--------
 test/lisp/simple-tests.el | 17 +++++++++++++++--
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b3a044d..e76628a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -187,6 +187,12 @@ questions, with a handy way to display help texts.
 'undo', undo the last replacement; bound to 'u'.
 'undo-all', undo all replacements; bound to 'U'.
 
+** 'delete-trailing-whitespace' deletes whitespace after form feed.
+In modes where form feed was treated as a whitespace character,
+'delete-trailing-whitespace' would keep lines containing it unchanged.
+It now deletes whitespace after the last form feed thus behaving the
+same as in modes where the character is not whitespace.
+
 \f
 * Changes in Specialized Modes and Packages in Emacs 25.2
 
diff --git a/lisp/simple.el b/lisp/simple.el
index bc3e7b8..ffedbfa 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -602,15 +602,14 @@ delete-trailing-whitespace
                    (list nil nil))))
   (save-match-data
     (save-excursion
-      (let ((end-marker (copy-marker (or end (point-max))))
-            (start (or start (point-min))))
-        (goto-char start)
-        (while (re-search-forward "\\s-$" end-marker t)
-          (skip-syntax-backward "-" (line-beginning-position))
+      (let ((end-marker (copy-marker (or end (point-max)))))
+        (goto-char (or start (point-min)))
+        (with-syntax-table (make-syntax-table (syntax-table))
           ;; 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)))
+          (modify-syntax-entry ?\f "_")
+          (while (re-search-forward "\\s-$" end-marker t)
+            (skip-syntax-backward "-" (line-beginning-position))
+            (delete-region (point) (match-end 0))))
         ;; Delete trailing empty lines.
         (goto-char end-marker)
         (when (and (not end)
diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index 40cd1d2..2722544 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -204,7 +204,7 @@ simple-test--transpositions
 
 \f
 ;;; `delete-trailing-whitespace'
-(ert-deftest simple-delete-trailing-whitespace ()
+(ert-deftest simple-delete-trailing-whitespace--bug-21766 ()
   "Test bug#21766: delete-whitespace sometimes deletes non-whitespace."
   (defvar python-indent-guess-indent-offset)  ; to avoid a warning
   (let ((python (featurep 'python))
@@ -219,11 +219,24 @@ simple-test--transpositions
                           "\n"
                           "\n"))
           (delete-trailing-whitespace)
-          (should (equal (count-lines (point-min) (point-max)) 3)))
+          (should (string-equal (buffer-string)
+                                (concat "query = \"\"\"WITH filtered AS\n"
+                                        "WHERE\n"
+                                        "\"\"\".format(fv_)\n"))))
       ;; Let's clean up if running interactive
       (unless (or noninteractive python)
         (unload-feature 'python)))))
 
+(ert-deftest simple-delete-trailing-whitespace--formfeeds ()
+  "Test formfeeds are not deleted but whitespace past them is."
+  (with-temp-buffer
+    (with-syntax-table (make-syntax-table)
+      (modify-syntax-entry ?\f " ")     ; Make sure \f is whitespace
+      (insert " \f \n \f \f \n\nlast\n")
+      (delete-trailing-whitespace)
+      (should (string-equal (buffer-string) " \f\n \f \f\n\nlast\n"))
+      (should (equal ?\s (char-syntax ?\f))))))
+
 
 ;;; auto-boundary tests
 (ert-deftest undo-auto-boundary-timer ()
-- 
2.8.0.rc3.226.g39d4020

bug#23843: [PATCH 2/3] Simplify ‘delete-trailing-whitespace’ by not treating \n as whitespace

[Michal Nazarewicz]

* lisp/simple.el (delete-trailing-whitespace): Set newline’s character
syntax to non-whitespace so that ‘\s-’ regular expression does not match
it.

This simplifies the loop slightly since a simple ‘\s-+$’ can be used and
as a consequence ‘line-beginning-position’ function does not need to be
called any longer.

Furthermore, when newline has whitespace syntax, ‘\s-$’ regular
expression ends up matching empty lins since ‘\s-’ matches newline
characetr of proceeding line.  This leads to needless loop iterations.

Since previous change to ‘delete-trailing-whitespace’ already introduced
‘with-syntax-table’, take advantage of it and also overwrite newline’s
character syntax.
---
 lisp/simple.el            | 7 ++++---
 test/lisp/simple-tests.el | 3 ++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index ffedbfa..fbeef9d 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -607,9 +607,10 @@ delete-trailing-whitespace
         (with-syntax-table (make-syntax-table (syntax-table))
           ;; Don't delete formfeeds, even if they are considered whitespace.
           (modify-syntax-entry ?\f "_")
-          (while (re-search-forward "\\s-$" end-marker t)
-            (skip-syntax-backward "-" (line-beginning-position))
-            (delete-region (point) (match-end 0))))
+          ;; Treating \n as non-whitespace makes things easier.
+          (modify-syntax-entry ?\n "_")
+          (while (re-search-forward "\\s-+$" end-marker t)
+            (delete-region (match-beginning 0) (match-end 0))))
         ;; Delete trailing empty lines.
         (goto-char end-marker)
         (when (and (not end)
diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index 2722544..97b6c49 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -235,7 +235,8 @@ simple-test--transpositions
       (insert " \f \n \f \f \n\nlast\n")
       (delete-trailing-whitespace)
       (should (string-equal (buffer-string) " \f\n \f \f\n\nlast\n"))
-      (should (equal ?\s (char-syntax ?\f))))))
+      (should (equal ?\s (char-syntax ?\f)))
+      (should (equal ?\s (char-syntax ?\n))))))
 
 
 ;;; auto-boundary tests
-- 
2.8.0.rc3.226.g39d4020

bug#23843: [PATCH 3/3] Don’t create unnecessary marker in ‘delete-trailing-whitespace’

[Michal Nazarewicz]

* lisp/simple.el (delete-trailing-whitespace): If END argument is nil,
there is no need for the end-marker to be created.
---
 lisp/simple.el | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index fbeef9d..6e440fc 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -602,7 +602,7 @@ delete-trailing-whitespace
                    (list nil nil))))
   (save-match-data
     (save-excursion
-      (let ((end-marker (copy-marker (or end (point-max)))))
+      (let ((end-marker (and end (copy-marker end))))
         (goto-char (or start (point-min)))
         (with-syntax-table (make-syntax-table (syntax-table))
           ;; Don't delete formfeeds, even if they are considered whitespace.
@@ -611,15 +611,14 @@ delete-trailing-whitespace
           (modify-syntax-entry ?\n "_")
           (while (re-search-forward "\\s-+$" end-marker t)
             (delete-region (match-beginning 0) (match-end 0))))
-        ;; Delete trailing empty lines.
-        (goto-char end-marker)
-        (when (and (not end)
-		   delete-trailing-lines
-                   ;; Really the end of buffer.
-		   (= (point-max) (1+ (buffer-size)))
-                   (<= (skip-chars-backward "\n") -2))
-          (delete-region (1+ (point)) end-marker))
-        (set-marker end-marker nil))))
+        (if end
+            (set-marker end-marker nil)
+          ;; Delete trailing empty lines.
+          (and delete-trailing-lines
+               ;; Really the end of buffer.
+               (= (goto-char (point-max)) (1+ (buffer-size)))
+               (<= (skip-chars-backward "\n") -2)
+               (delete-region (1+ (point)) (point-max)))))))
   ;; Return nil for the benefit of `write-file-functions'.
   nil)
 
-- 
2.8.0.rc3.226.g39d4020

bug#23843: Small `delete-trailing-whitespace' fixes and improvements

[Michal Nazarewicz]

Pushed.

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

bug#23843: Small `delete-trailing-whitespace' fixes and improvements

[Glenn Morris]

Michal Nazarewicz wrote:

> Pushed.

You seem to have made an unfortunate typo and somehow created a new
branch "masater" with this change.

bug#23843: Small `delete-trailing-whitespace' fixes and improvements

[Michal Nazarewicz]

> Michal Nazarewicz wrote:
>> Pushed.

On Mon, Jul 04 2016, Glenn Morris wrote:
> You seem to have made an unfortunate typo and somehow created a new
> branch "masater" with this change.

Yes, sorry.  This has been fixed.

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»