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

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

[Jon Irving]

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

In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.7)
 of 2019-03-26 built on odo
Repository revision: c8ec3108a3d0bd1955d21f40b3c0c3b36d55b20d
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Arch Linux

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY INOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LIBSYSTEMD JSON PDUMPER
LCMS2 GMP

Important settings:
  value of $LC_MESSAGES: 
  value of $LC_MONETARY: en_CA.UTF-8
  value of $LC_NUMERIC: en_CA.UTF-8
  value of $LC_TIME: en_CA.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
easymenu mml-sec password-cache epa derived epg epg-config gnus-util
rmail rmail-loaddefs text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
elec-pair mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 44928 6804)
 (symbols 48 5888 1)
 (strings 32 15000 1709)
 (string-bytes 1 494536)
 (vectors 16 9510)
 (vector-slots 8 117542 10312)
 (floats 8 17 47)
 (intervals 56 189 0)
 (buffers 992 11))

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

[Basil L. Contovounesios]

[-- 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

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

[Jon Irving]

Hi Basil

>>>>>> Basil L. Contovounesios <contovob@tcd.ie> at 2019-03-27T12:09:14-0400:

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

I just installed this patch on a freshly updated clone of master, and
can confirm the bug is no longer evident. So from my perspective this is
definitely acceptable.

Thanks for taking care of this so quickly!

cheers,
Jon
-- 
Jonathan Irving
https://j0ni.ca
https://keybase.io/j0ni

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

[Basil L. Contovounesios]

Jon Irving <j@lollyshouse.ca> writes:

>>>>>>> Basil L. Contovounesios <contovob@tcd.ie> at 2019-03-27T12:09:14-0400:
>
>   > I attach a patch which fixes this and also updates the entry for
>   > delete-indentation in the Elisp manual.  Is it acceptable?
>
> I just installed this patch on a freshly updated clone of master, and
> can confirm the bug is no longer evident. So from my perspective this is
> definitely acceptable.
>
> Thanks for taking care of this so quickly!

No worries, thanks for the report and confirming the fix!

-- 
Basil

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Wed, 27 Mar 2019 16:09:14 +0000
> Cc: 35021@debbugs.gnu.org, Stephen Leake <stephen_leake@stephe-leake.org>,
> 	Łukasz Stelmach <stlman@poczta.fm>
> 
> > 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?

Looks OK to me, please push.  Alex, this will also solve your bug
report, right?  If now, why not?

Thanks.

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

[Basil L. Contovounesios]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Fix-recently-extended-delete-indentation-behavior.patch --]
[-- Type: text/x-diff, Size: 13440 bytes --]

From f0bead89007c102754d49305c9669ffe1bcc25d0 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 recently extended delete-indentation behavior

* doc/lispref/text.texi (User-Level Deletion): Document new optional
arguments of delete-indentation.

* lisp/simple.el (delete-indentation): Do not barf if called
interactively when region is inactive. (bug#35021)
Do not skip blank lines. (bug#35036)
Consistently deactivate mark even when no text was changed.
Handle active region spanning a single line.

* test/lisp/simple-tests.el (simple-test--buffer-substrings):
New convenience function.
(simple-test--dummy-buffer, simple-test--transpositions): Use it.
(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.
(simple-delete-indentation-blank-line)
(simple-delete-indentation-boundaries)
(simple-delete-indentation-region)
(simple-delete-indentation-prefix): New tests.
---
 doc/lispref/text.texi     |  10 ++-
 lisp/simple.el            |  54 +++++++-----
 test/lisp/simple-tests.el | 176 ++++++++++++++++++++++++++++++--------
 3 files changed, 181 insertions(+), 59 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..b87c6ef052 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -598,30 +598,38 @@ 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)))
-  (beginning-of-line)
-  (while (eq (preceding-char) ?\n)
-    (progn
-      (delete-region (point) (1- (point)))
-      ;; If the second line started with the fill prefix,
+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))))))
+  ;; Consistently deactivate mark even when no text was changed.
+  (setq deactivate-mark t)
+  (if (and beg (not arg))
+      ;; Region is active.  Go to END, but only if region spans
+      ;; multiple lines.
+      (and (goto-char beg)
+           (> end (line-end-position))
+           (goto-char end))
+    ;; Region is inactive.  Set a loop sentinel
+    ;; (subtracting 1 in order to compare less than BOB).
+    (setq beg (1- (line-beginning-position (and arg 2))))
+    (when arg (forward-line)))
+  (let ((prefix (and (> (length fill-prefix) 0)
+                     (regexp-quote fill-prefix))))
+    (while (and (< beg (line-beginning-position))
+                (forward-line 0)
+                (= (preceding-char) ?\n))
+      (delete-char -1)
+      ;; If the appended 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))))
-      (fixup-whitespace)
-      (if (and (use-region-p)
-               beg
-               (not arg)
-	       (< beg (point-at-bol)))
-	  (beginning-of-line)))))
+      (if (and prefix (looking-at prefix))
+          (replace-match "" t t))
+      (fixup-whitespace))))
 
 (defalias 'join-line #'delete-indentation) ; easier to find
 
diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index d9f059c8fc..cc2feebbef 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -22,6 +22,11 @@
 (require 'ert)
 (eval-when-compile (require 'cl-lib))
 
+(defun simple-test--buffer-substrings ()
+  "Return cons of buffer substrings before and after point."
+  (cons (buffer-substring (point-min) (point))
+        (buffer-substring (point) (point-max))))
+
 (defmacro simple-test--dummy-buffer (&rest body)
   (declare (indent 0)
            (debug t))
@@ -31,10 +36,7 @@ simple-test--dummy-buffer
      (insert "(a b")
      (save-excursion (insert " c d)"))
      ,@body
-     (with-no-warnings
-       (cons (buffer-substring (point-min) (point))
-             (buffer-substring (point) (point-max))))))
-
+     (with-no-warnings (simple-test--buffer-substrings))))
 
 \f
 ;;; `transpose-sexps'
@@ -46,8 +48,7 @@ simple-test--transpositions
      (insert "(s1) (s2) (s3) (s4) (s5)")
      (backward-sexp 1)
      ,@body
-     (cons (buffer-substring (point-min) (point))
-           (buffer-substring (point) (point-max)))))
+     (simple-test--buffer-substrings)))
 
 ;;; Transposition with negative args (bug#20698, bug#21885)
 (ert-deftest simple-transpose-subr ()
@@ -215,37 +216,144 @@ 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")))
-        )))
+  "Test `delete-indentation' when no mark is set; see bug#35021."
+  (with-temp-buffer
+    (insert " first \n second \n third \n fourth ")
+    (should-not (mark t))
+    ;; Without prefix argument.
+    (should-not (call-interactively #'delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first \n second \n third" . " fourth ")))
+    (should-not (call-interactively #'delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first \n second" . " third fourth ")))
+    ;; With prefix argument.
+    (goto-char (point-min))
+    (let ((current-prefix-arg '(4)))
+      (should-not (call-interactively #'delete-indentation)))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first" . " second third fourth ")))))
 
 (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")))
-        )))
+  "Test `delete-indentation'  with an inactive region."
+  (with-temp-buffer
+    (insert " first \n second \n third ")
+    (set-marker (mark-marker) (point-min))
+    (should (mark t))
+    (should-not (call-interactively #'delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first \n second" . " third ")))))
+
+(ert-deftest simple-delete-indentation-blank-line ()
+  "Test `delete-indentation' does not skip blank lines.
+See bug#35036."
+  (with-temp-buffer
+    (insert "\n\n third \n \n \n sixth \n\n")
+    ;; Without prefix argument.
+    (should-not (delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '("\n\n third \n \n \n sixth \n" . "")))
+    (should-not (delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '("\n\n third \n \n \n sixth" . "")))
+    (should-not (delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '("\n\n third \n \n" . "sixth")))
+    ;; With prefix argument.
+    (goto-char (point-min))
+    (should-not (delete-indentation t))
+    (should (equal (simple-test--buffer-substrings)
+                   '("" . "\n third \n \nsixth")))
+    (should-not (delete-indentation t))
+    (should (equal (simple-test--buffer-substrings)
+                   '("" . "third \n \nsixth")))
+    (should-not (delete-indentation t))
+    (should (equal (simple-test--buffer-substrings)
+                   '("third" . "\nsixth")))
+    (should-not (delete-indentation t))
+    (should (equal (simple-test--buffer-substrings)
+                   '("third" . " sixth")))))
+
+(ert-deftest simple-delete-indentation-boundaries ()
+  "Test `delete-indentation' motion at buffer boundaries."
+  (with-temp-buffer
+    (insert " first \n second \n third ")
+    ;; Stay at EOB.
+    (should-not (delete-indentation t))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first \n second \n third " . "")))
+    ;; Stay at BOB.
+    (forward-line -1)
+    (save-restriction
+      (narrow-to-region (point) (line-end-position))
+      (should-not (delete-indentation))
+      (should (equal (simple-test--buffer-substrings)
+                     '("" . " second ")))
+      ;; Go to EOB.
+      (should-not (delete-indentation t))
+      (should (equal (simple-test--buffer-substrings)
+                     '(" second " . ""))))
+    ;; Go to BOB.
+    (end-of-line 0)
+    (should-not (delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '("" . " first \n second \n third ")))))
+
+(ert-deftest simple-delete-indentation-region ()
+  "Test `delete-indentation' with an active region."
+  (with-temp-buffer
+    ;; Empty region.
+    (insert " first ")
+    (should-not (delete-indentation nil (point) (point)))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first " . "")))
+    ;; Single line.
+    (should-not (delete-indentation
+                 nil (line-beginning-position) (1- (point))))
+    (should (equal (simple-test--buffer-substrings)
+                   '("" . " first ")))
+    (should-not (delete-indentation nil (1+ (point)) (line-end-position)))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" " . "first ")))
+    (should-not (delete-indentation
+                 nil (line-beginning-position) (line-end-position)))
+    (should (equal (simple-test--buffer-substrings)
+                   '("" . " first ")))
+    ;; Multiple lines.
+    (goto-char (point-max))
+    (insert "\n second \n third \n fourth ")
+    (goto-char (point-min))
+    (should-not (delete-indentation
+                 nil (line-end-position) (line-beginning-position 2)))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first" . " second \n third \n fourth ")))
+    (should-not (delete-indentation
+                 nil (point) (1+ (line-beginning-position 2))))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first second" . " third \n fourth ")))
+    ;; Prefix argument overrides region.
+    (should-not (delete-indentation t (point-min) (point)))
+    (should (equal (simple-test--buffer-substrings)
+                   '(" first second third" . " fourth ")))))
+
+(ert-deftest simple-delete-indentation-prefix ()
+  "Test `delete-indentation' with a fill prefix."
+  (with-temp-buffer
+    (insert "> first \n> second \n> third \n> fourth ")
+    (let ((fill-prefix ""))
+      (delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '("> first \n> second \n> third" . " > fourth ")))
+    (let ((fill-prefix "<"))
+      (delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '("> first \n> second" . " > third > fourth ")))
+    (let ((fill-prefix ">"))
+      (delete-indentation))
+    (should (equal (simple-test--buffer-substrings)
+                   '("> first" . " second > third > fourth ")))))
 
 \f
 ;;; `delete-trailing-whitespace'
-- 
2.20.1


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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Date: Wed, 27 Mar 2019 16:09:14 +0000
>> Cc: 35021@debbugs.gnu.org, Stephen Leake <stephen_leake@stephe-leake.org>,
>> 	Łukasz Stelmach <stlman@poczta.fm>
>> 
>> > 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?
>
> Looks OK to me, please push.

Thanks, but my last patch only fixed the bug with the interactive spec,
and I have since discovered and fixed a few more edge-cases (including
that reported by Alex in bug#35036).

I attach the updated patch.  Can I push this one instead?

> Alex, this will also solve your bug report, right?  If now, why not?

My last patch didn't address bug#35036.  Whereas bug#35021 is caused by
a wrong interactive spec, bug#35036 is caused by a wrong loop condition.

The attached patch should fix both issues (and a couple more).

Thanks,

-- 
Basil

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

[Alex Branham]

On Sat 30 Mar 2019 at 21:41, Basil L. Contovounesios <contovob@tcd.ie> wrote:

> The attached patch should fix both issues (and a couple more).

This does indeed seem to fix my issue, thanks for working on it!

Alex

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Alex Branham <alex.branham@gmail.com>,  <j@lollyshouse.ca>,  <35021@debbugs.gnu.org>,  <stephen_leake@stephe-leake.org>,  <stlman@poczta.fm>
> Date: Sun, 31 Mar 2019 03:41:47 +0100
> 
> > Looks OK to me, please push.
> 
> Thanks, but my last patch only fixed the bug with the interactive spec,
> and I have since discovered and fixed a few more edge-cases (including
> that reported by Alex in bug#35036).
> 
> I attach the updated patch.  Can I push this one instead?

Yes, this LGTM as well.

> > Alex, this will also solve your bug report, right?  If now, why not?
> 
> My last patch didn't address bug#35036.  Whereas bug#35021 is caused by
> a wrong interactive spec, bug#35036 is caused by a wrong loop condition.
> 
> The attached patch should fix both issues (and a couple more).

Thanks.

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

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: Alex Branham <alex.branham@gmail.com>,  <j@lollyshouse.ca>,  <35021@debbugs.gnu.org>,  <stephen_leake@stephe-leake.org>,  <stlman@poczta.fm>
>> Date: Sun, 31 Mar 2019 03:41:47 +0100
>> 
>> > Looks OK to me, please push.
>> 
>> Thanks, but my last patch only fixed the bug with the interactive spec,
>> and I have since discovered and fixed a few more edge-cases (including
>> that reported by Alex in bug#35036).
>> 
>> I attach the updated patch.  Can I push this one instead?
>
> Yes, this LGTM as well.

Thanks, I pushed to master and am thus closing this report.

-- 
Basil

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

[Basil L. Contovounesios]

Alex Branham <alex.branham@gmail.com> writes:

> On Sat 30 Mar 2019 at 21:41, Basil L. Contovounesios <contovob@tcd.ie> wrote:
>
>> The attached patch should fix both issues (and a couple more).
>
> This does indeed seem to fix my issue, thanks for working on it!

Thanks for reporting and testing!

-- 
Basil