bug#35021: M-^ (delete-indentation) doesn't work without a mark present

["Basil L. Contovounesios" <contovob@tcd.ie>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Fix-delete-indentation-when-region-is-inactive.patch --]
[-- Type: text/x-diff, Size: 6209 bytes --]

From 80cde7b9e392e81d70dc338a4dfa4bfc12f54103 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Wed, 27 Mar 2019 15:13:25 +0000
Subject: [PATCH] Fix delete-indentation when region is inactive

* lisp/simple.el (delete-indentation): Do not barf if called
interactively when region is inactive. (bug#35021)

* test/lisp/simple-tests.el (simple-delete-indentation-no-region):
(simple-delete-indentation-inactive-region): Update commentary.
Call delete-indentation interactively when testing for behavior with
inactive region and region is not explicitly defined.

* doc/lispref/text.texi (User-Level Deletion): Document new optional
arguments of delete-indentation.
---
 doc/lispref/text.texi     | 10 ++++++--
 lisp/simple.el            | 36 ++++++++++++++--------------
 test/lisp/simple-tests.el | 50 +++++++++++++++++----------------------
 3 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 21c5a73f88..b430adf597 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -723,12 +723,18 @@ User-Level Deletion
 @end example
 @end deffn
 
-@deffn Command delete-indentation &optional join-following-p
+@deffn Command delete-indentation &optional join-following-p beg end
 This function joins the line point is on to the previous line, deleting
 any whitespace at the join and in some cases replacing it with one
 space.  If @var{join-following-p} is non-@code{nil},
 @code{delete-indentation} joins this line to the following line
-instead.  The function returns @code{nil}.
+instead.  Otherwise, if @var{beg} and @var{end} are non-@code{nil},
+this function joins all lines in the region they define.
+
+In an interactive call, @var{join-following-p} is the prefix argument,
+and @var{beg} and @var{end} are, respectively, the start and end of
+the region if it is active, else @code{nil}.  The function returns
+@code{nil}.
 
 If there is a fill prefix, and the second of the lines being joined
 starts with the prefix, then @code{delete-indentation} deletes the
diff --git a/lisp/simple.el b/lisp/simple.el
index f76f31ad14..6738df3cb9 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -598,29 +598,29 @@ delete-indentation
 If there is a fill prefix, delete it from the beginning of this
 line.
 With prefix ARG, join the current line to the following line.
-If the region is active, join all the lines in the region.  (The
-region is ignored if prefix argument is given.)"
-  (interactive "*P\nr")
-  (if arg (forward-line 1)
-    (if (use-region-p)
-	(goto-char end)))
+When BEG and END are non-nil, join all lines in the region they
+define.  Interactively, BEG and END are, respectively, the start
+and end of the region if it is active, else nil.  (The region is
+ignored if prefix ARG is given.)"
+  (interactive
+   (progn (barf-if-buffer-read-only)
+          (cons current-prefix-arg
+                (and (use-region-p)
+                     (list (region-beginning) (region-end))))))
+  (cond (arg (forward-line 1))
+        (end (goto-char end)))
   (beginning-of-line)
-  (while (eq (preceding-char) ?\n)
-    (progn
-      (delete-region (point) (1- (point)))
+  (let ((prefix (and fill-prefix (regexp-quote fill-prefix))))
+    (while (= (preceding-char) ?\n)
+      (delete-char -1)
       ;; If the second line started with the fill prefix,
       ;; delete the prefix.
-      (if (and fill-prefix
-	       (<= (+ (point) (length fill-prefix)) (point-max))
-	       (string= fill-prefix
-			(buffer-substring (point)
-					  (+ (point) (length fill-prefix)))))
-	  (delete-region (point) (+ (point) (length fill-prefix))))
+      (if (and prefix (looking-at prefix))
+          (replace-match "" t t))
       (fixup-whitespace)
-      (if (and (use-region-p)
-               beg
+      (if (and beg
                (not arg)
-	       (< beg (point-at-bol)))
+               (< beg (line-beginning-position)))
 	  (beginning-of-line)))))
 
 (defalias 'join-line #'delete-indentation) ; easier to find
diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index d9f059c8fc..f80578c673 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -216,36 +216,30 @@ simple-test--transpositions
 \f
 ;;; `delete-indentation'
 (ert-deftest simple-delete-indentation-no-region ()
-  "delete-indentation works when no mark is set."
-  ;; interactive \r returns nil for BEG END args
-  (unwind-protect
-      (with-temp-buffer
-        (insert (concat "zero line \n"
-                        "first line \n"
-                        "second line"))
-        (delete-indentation)
-        (should (string-equal
-                 (buffer-string)
-                 (concat "zero line \n"
-                         "first line second line")))
-        )))
+  "`delete-indentation' works when no mark is set."
+  (with-temp-buffer
+    (insert "first line \n"
+            "second line \n"
+            "third line")
+    (call-interactively #'delete-indentation)
+    (should (string-equal
+             (buffer-string)
+             (concat "first line \n"
+                     "second line third line")))))
 
 (ert-deftest simple-delete-indentation-inactive-region ()
-  "delete-indentation ignores inactive region."
-  ;; interactive \r returns non-nil for BEG END args
-  (unwind-protect
-      (with-temp-buffer
-        (insert (concat "zero line \n"
-                        "first line \n"
-                        "second line"))
-        (push-mark (point-min) t t)
-        (deactivate-mark)
-        (delete-indentation)
-        (should (string-equal
-                 (buffer-string)
-                 (concat "zero line \n"
-                         "first line second line")))
-        )))
+  "`delete-indentation' ignores inactive region."
+  (with-temp-buffer
+    (insert "first line \n"
+            "second line \n"
+            "third line")
+    (push-mark (point-min) t t)
+    (deactivate-mark)
+    (call-interactively #'delete-indentation)
+    (should (string-equal
+             (buffer-string)
+             (concat "first line \n"
+                     "second line third line")))))
 
 \f
 ;;; `delete-trailing-whitespace'
-- 
2.20.1


[-- Attachment #2: Type: text/plain, Size: 956 bytes --]


Jon Irving <j@lollyshouse.ca> writes:

> I believe this is related to the following commits:
>
> b515edb985 Fix bug in delete-indentation when region is inactive
> 09c220a5cf Minor fixes for the last change
> 8fa94a1ecc If the region is active, join all the lines it spans
>
> From a clean `emacs -Q` start:
>
> 1. Move point up to the bottom line of the *scratch* buffer comments
> 2. Type M-^ (or M-x delete-indentation)
> 3. Observe the following message in the minibuffer:
>      The mark is not set now, so there is no region

This is because delete-indentation is currently using the 'r'
interactive code, which barfs if the region is inactive.

I attach a patch which fixes this and also updates the entry for
delete-indentation in the Elisp manual.  Is it acceptable?

Stephen, what is the difference between the two tests
simple-delete-indentation-no-region and
simple-delete-indentation-inactive-region?  Can they be merged?

Thanks,

-- 
Basil