bug#13521: `sort-lines' on the empty region

bug#13521: `sort-lines' on the empty region

[Xue Fuqiao]

The current behavior of `sort-lines' on the empty region is to do
nothing. I often find that I want to sort whole buffers at the time,
and I expect `sort-lines' to do just that, before remembering I have
to mark the whole buffer first.

Wouldn't it be convenient to change the behavior of `sort-lines' so
that it sorts the whole buffer if no region is marked? Or is this
inconsistent with how other region-operating functions works?

See:
http://lists.gnu.org/archive/html/emacs-devel/2013-01/msg00492.html
-- 
Best regards, Xue Fuqiao.
http://www.emacswiki.org/emacs/XueFuqiao

bug#13521: `sort-lines' on the empty region

[Glenn Morris]

Please wait a week or so before reposting something from one Emacs list
on another, to allow time for people to reply.

Xue Fuqiao wrote:

> http://lists.gnu.org/archive/html/emacs-devel/2013-01/msg00492.html

See the very first reply, posted before this report.

bug#13521: `sort-lines' on the empty region

[Xue Fuqiao]

On Mon, 21 Jan 2013 22:19:12 -0500
Glenn Morris <rgm@gnu.org> wrote:

> Please wait a week or so before reposting something from one Emacs list
> on another, to allow time for people to reply.
Thanks for your advice.

> Xue Fuqiao wrote:
> 
> > http://lists.gnu.org/archive/html/emacs-devel/2013-01/msg00492.html
> 
> See the very first reply, posted before this report.
I've seen this reply, but I'd like to report it as a wishlist.

-- 
Best regards, Xue Fuqiao.
http://www.emacswiki.org/emacs/XueFuqiao

bug#13521: `sort-lines' on the empty region

[Xue Fuqiao]

On Mon, 21 Jan 2013 22:19:12 -0500
Glenn Morris <rgm@gnu.org> wrote:

> Please wait a week or so before reposting something from one Emacs list
> on another, to allow time for people to reply.
Thanks for your advice.

> Xue Fuqiao wrote:
> 
> > http://lists.gnu.org/archive/html/emacs-devel/2013-01/msg00492.html
> 
> See the very first reply, posted before this report.
I've seen this reply, but I'd like to report it as a wishlist.

-- 
Best regards, Xue Fuqiao.
http://www.emacswiki.org/emacs/XueFuqiao

bug#13521: `sort-lines' on the empty region

[Stefan Monnier]

> The current behavior of `sort-lines' on the empty region is to do
> nothing.

I think that's the right thing to do.

> I often find that I want to sort whole buffers at the time,
> and I expect `sort-lines' to do just that, before remembering I have
> to mark the whole buffer first.

That seems to be considering a different case: not the case of an empty
region, but the case where there's no active region.
Currently, this case signals an error, but I think I agree it makes
sense to let it apply to the whole buffer if there is no active region.


        Stefan

bug#13521: `sort-lines' on the empty region

[Glenn Morris]

Stefan Monnier wrote:

> That seems to be considering a different case: not the case of an empty
> region, but the case where there's no active region.
> Currently, this case signals an error, but I think I agree it makes
> sense to let it apply to the whole buffer if there is no active region.

flush-lines, keep-lines instead operate on all lines after point.

bug#13521: `sort-lines' on the empty region

[Stefan Monnier]

>> That seems to be considering a different case: not the case of an empty
>> region, but the case where there's no active region.
>> Currently, this case signals an error, but I think I agree it makes
>> sense to let it apply to the whole buffer if there is no active region.
> flush-lines, keep-lines instead operate on all lines after point.

Yes, we have an inconsistency in this respect: some commands use "the
whole buffer" and others use "everything after point".  Whichever choice
we make for sort-lines, it will be inconsistent with some of the
existing commands ;-)

I personally prefer the "whole buffer", but whoever makes the change
gets to make the choice.


        Stefan

bug#13521: `sort-lines' on the empty region

[Stefan Kangas]

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

tags 13521 + patch
thanks

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> That seems to be considering a different case: not the case of an empty
>>> region, but the case where there's no active region.
>>> Currently, this case signals an error, but I think I agree it makes
>>> sense to let it apply to the whole buffer if there is no active region.
>> flush-lines, keep-lines instead operate on all lines after point.
>
> Yes, we have an inconsistency in this respect: some commands use "the
> whole buffer" and others use "everything after point".  Whichever choice
> we make for sort-lines, it will be inconsistent with some of the
> existing commands ;-)
>
> I personally prefer the "whole buffer", but whoever makes the change
> gets to make the choice.

The attached patch makes 'sort-lines' sort the entire buffer when there
is no region.

Any comments?

Best regards,
Stefan Kangas

[-- Attachment #2: 0001-Make-sort-lines-sort-entire-buffer-without-active-re.patch --]
[-- Type: text/x-diff, Size: 2693 bytes --]

From 6c1d6eb1dc8306f99fd01ba848213632ac6cd973 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Fri, 21 Aug 2020 03:06:16 +0200
Subject: [PATCH] Make 'sort-lines' sort entire buffer without active region

* lisp/sort.el (sort-lines): Sort entire buffer if there is no active
region.  Doc fix.  (Bug#13521)
* test/lisp/sort-tests.el (sort-tests--lines/entire-buffer): New test.
* etc/NEWS: Announce the above change.
---
 etc/NEWS                |  3 +++
 lisp/sort.el            | 12 +++++++++++-
 test/lisp/sort-tests.el |  6 ++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 53391f91f7..1a21525328 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -145,6 +145,9 @@ setting the variable 'auto-save-visited-mode' buffer-locally to nil.
 description of the properties.  Likewise 'button-describe' does the
 same for a button.
 
+---
+** 'sort-lines' will now sort the entire buffer if there is no region.
+
 \f
 * Changes in Specialized Modes and Packages in Emacs 28.1
 
diff --git a/lisp/sort.el b/lisp/sort.el
index f878db24a3..b1cade4266 100644
--- a/lisp/sort.el
+++ b/lisp/sort.el
@@ -200,11 +200,21 @@ sort-reorder-buffer
 (defun sort-lines (reverse beg end)
   "Sort lines in region alphabetically; REVERSE non-nil means descending order.
 Interactively, REVERSE is the prefix argument, and BEG and END are the region.
+If there is no region selected, sort the entire buffer.
+
 Called from a program, there are three arguments:
 REVERSE (non-nil means reverse order), BEG and END (region to sort).
 The variable `sort-fold-case' determines whether alphabetic case affects
 the sort order."
-  (interactive "P\nr")
+  (interactive
+   (progn
+     (barf-if-buffer-read-only)
+     (list current-prefix-arg nil nil)))
+  (if (and (called-interactively-p 'any) (use-region-p))
+      (setq beg (region-beginning)
+            end (region-end))
+    (setq beg (point-min)
+          end (point-max)))
   (save-excursion
     (save-restriction
       (narrow-to-region beg end)
diff --git a/test/lisp/sort-tests.el b/test/lisp/sort-tests.el
index 21f483a23a..3c6f0a0024 100644
--- a/test/lisp/sort-tests.el
+++ b/test/lisp/sort-tests.el
@@ -78,6 +78,12 @@ sort-tests-test-sorter-function
 (ert-deftest sort-tests--lines ()
   (sort-tests-test-sorter-function "\n" #'sort-lines))
 
+(ert-deftest sort-tests--lines/entire-buffer ()
+  (with-temp-buffer
+    (insert "C\nA\nB")
+    (call-interactively #'sort-lines)
+    (should (equal (buffer-string) "A\nB\nC"))))
+
 (ert-deftest sort-tests--paragraphs ()
   (let ((paragraph-separate "[\s\t\f]*$"))
     (sort-tests-test-sorter-function "\n\n" #'sort-paragraphs)))
-- 
2.28.0

bug#13521: `sort-lines' on the empty region

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 20 Aug 2020 18:18:34 -0700
> Cc: Xue Fuqiao <xfq.free@gmail.com>, Glenn Morris <rgm@gnu.org>,
>  13521@debbugs.gnu.org
> 
> > I personally prefer the "whole buffer", but whoever makes the change
> > gets to make the choice.
> 
> The attached patch makes 'sort-lines' sort the entire buffer when there
> is no region.

How frequently did you see a buffer in Emacs with no region in it?
IME, it takes about 2 commands since buffer creation to have a region
in it.  So I predict users to be tripped by this feature quite a lot:
they will think there's no region, invoke the command, and get some
arbitrary region sorted.  Requiring them to always provide the region
avoids this pitfall, since the user is forced to make sure the region
is where he or she wants it.  And marking the whole buffer is just 2
key-presses away.

So I think this change will annoy more than help.

bug#13521: `sort-lines' on the empty region

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> The attached patch makes 'sort-lines' sort the entire buffer when there
>> is no region.
>
> How frequently did you see a buffer in Emacs with no region in it?
> IME, it takes about 2 commands since buffer creation to have a region
> in it.  So I predict users to be tripped by this feature quite a lot:
> they will think there's no region, invoke the command, and get some
> arbitrary region sorted.  Requiring them to always provide the region
> avoids this pitfall, since the user is forced to make sure the region
> is where he or she wants it.  And marking the whole buffer is just 2
> key-presses away.
>
> So I think this change will annoy more than help.

Sorry, the correct terminology I should have used is "active region".
This is as tested by `use-region-p'.  Compare the test I introduce also
to the one in `flush-lines', where I lifted it from.

Best regards,
Stefan Kangas

bug#13521: `sort-lines' on the empty region

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Fri, 21 Aug 2020 00:15:31 -0700
> Cc: monnier@iro.umontreal.ca, xfq.free@gmail.com, rgm@gnu.org, 
> 	13521@debbugs.gnu.org
> 
> > How frequently did you see a buffer in Emacs with no region in it?
> > IME, it takes about 2 commands since buffer creation to have a region
> > in it.  So I predict users to be tripped by this feature quite a lot:
> > they will think there's no region, invoke the command, and get some
> > arbitrary region sorted.  Requiring them to always provide the region
> > avoids this pitfall, since the user is forced to make sure the region
> > is where he or she wants it.  And marking the whole buffer is just 2
> > key-presses away.
> >
> > So I think this change will annoy more than help.
> 
> Sorry, the correct terminology I should have used is "active region".
> This is as tested by `use-region-p'.  Compare the test I introduce also
> to the one in `flush-lines', where I lifted it from.

So if the user has transient-mark-mode disabled (which is what I do),
the command will always sort the entire buffer?  Or did I
misunderstand something?

bug#13521: `sort-lines' on the empty region

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> So if the user has transient-mark-mode disabled (which is what I do),
> the command will always sort the entire buffer?  Or did I
> misunderstand something?

I didn't consider that case, but you are correct.  The behavior would be
like that documented in `flush-lines':

    Interactively, in Transient Mark mode when the mark is active, operate
    on the contents of the region.  Otherwise, operate from point to the
    end of (the accessible portion of) the buffer.

I'm not sure the `flush-lines' behavior makes sense, though.  It looks
like you can't run it on a region without transient-mark-mode.  So maybe
we should fix that as well.

(Note that `flush-lines' operates from point to point-max, but
`sort-lines' is proposed here to operate on the entire buffer instead as
per previous discussion in this bug.)

bug#13521: `sort-lines' on the empty region

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 5 Sep 2020 11:13:17 +0000
> Cc: xfq.free@gmail.com, rgm@gnu.org, monnier@iro.umontreal.ca, 
> 	13521@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So if the user has transient-mark-mode disabled (which is what I do),
> > the command will always sort the entire buffer?  Or did I
> > misunderstand something?
> 
> I didn't consider that case, but you are correct.  The behavior would be
> like that documented in `flush-lines':
> 
>     Interactively, in Transient Mark mode when the mark is active, operate
>     on the contents of the region.  Otherwise, operate from point to the
>     end of (the accessible portion of) the buffer.
> 
> I'm not sure the `flush-lines' behavior makes sense, though.  It looks
> like you can't run it on a region without transient-mark-mode.  So maybe
> we should fix that as well.

I think we should indeed fix flush-lines as well, yes.

> (Note that `flush-lines' operates from point to point-max, but
> `sort-lines' is proposed here to operate on the entire buffer instead as
> per previous discussion in this bug.)

I guess due to historical reasons?

bug#13521: `sort-lines' on the empty region

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> So I think this change will annoy more than help.

I agree -- I don't think we should change how `sort-lines' works here.
The current behaviour makes sense to me (and changing it sounds like it
might lead to people messing up their buffers), even if there are other
commands that do a DWIM-ish thing in this use case.

So I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#13521: `sort-lines' on the empty region

[Drew Adams]

> > > So if the user has transient-mark-mode disabled (which is what I do),
> > > the command will always sort the entire buffer?  Or did I
> > > misunderstand something?
> >
> > I didn't consider that case, but you are correct.  The behavior would be
> > like that documented in `flush-lines':
> >
> >     Interactively, in Transient Mark mode when the mark is active, operate
> >     on the contents of the region.  Otherwise, operate from point to the
> >     end of (the accessible portion of) the buffer.
> >
> > I'm not sure the `flush-lines' behavior makes sense, though.  It looks
> > like you can't run it on a region without transient-mark-mode.  So maybe
> > we should fix that as well.
> 
> I think we should indeed fix flush-lines as well, yes.
> 
> > (Note that `flush-lines' operates from point to point-max, but
> > `sort-lines' is proposed here to operate on the entire buffer instead as
> > per previous discussion in this bug.)
> 
> I guess due to historical reasons?

Apologies for not following this thread and perhaps
misunderstanding at this point.

If `transient-mark-mode' is off then there is no notion
of an "active region".  There is just the region.

I doubt that someone who has `transient-mark-mode' off
would ever want commands such as `flush-lines' and
`sort-lines' to act on the region.  And if they did, I
expect they'd just narrow to it.

Anything that works on "the active region" is something
that makes sense only when `transient-mark-mode' is on
(IMHO).

bug#13521: `sort-lines' on the empty region

[Eli Zaretskii]

> Date: Sat, 5 Sep 2020 07:36:55 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: xfq.free@gmail.com, rgm@gnu.org, monnier@iro.umontreal.ca,
>         13521@debbugs.gnu.org
> 
> I doubt that someone who has `transient-mark-mode' off
> would ever want commands such as `flush-lines' and
> `sort-lines' to act on the region.  And if they did, I
> expect they'd just narrow to it.

Please don't doubt, and please don't impose unnecessary commands on
users who have transient-mark-mode off.

> Anything that works on "the active region" is something
> that makes sense only when `transient-mark-mode' is on
> (IMHO).

The important point here is that sort-lines worked on the region, even
if inactive, before the proposed changes, so restricting it now only
to active regions would be a backward-incompatible change of behavior.

bug#13521: `sort-lines' on the empty region

[Stefan Monnier]

> The important point here is that sort-lines worked on the region, even
> if inactive, before the proposed changes, so restricting it now only
> to active regions would be a backward-incompatible change of behavior.

Indeed, so the patch should check something like

    (if (or (null transient-mark-mode) (use-region-p))
        (list (region-beginning) (region-end))
      (list (point-min) (point-max)))

`use-region-p` checks `transient-mark-mode` the other way, because it's
used mostly for commands which previously did *not* operate on the
region, unlike `sort-lines`.


        Stefan

bug#13521: `sort-lines' on the empty region

[Drew Adams]

> > I doubt that someone who has `transient-mark-mode' off
> > would ever want commands such as `flush-lines' and
> > `sort-lines' to act on the region.  And if they did, I
> > expect they'd just narrow to it.
> 
> Please don't doubt, and please don't impose unnecessary commands on
> users who have transient-mark-mode off.

Please don't claim that I imposed any commands on anyone.

The region is nearly always present and usually nonempty.
A user with `transient-mark-mode' off would typically
(IMHO) be bothered if `flush-lines' started always acting
on the region (it would be almost always: whenever there's
a mark in the buffer and the region is nonempty). 

Do you disagree?

> > Anything that works on "the active region" is something
> > that makes sense only when `transient-mark-mode' is on
> > (IMHO).
> 
> The important point here is that sort-lines worked on the region, even
> if inactive, before the proposed changes, so restricting it now only
> to active regions would be a backward-incompatible change of behavior.

Yes, sorry; I agree about `sort-lines'.  That is not
the case for `flush-lines' and `keep-lines'.

And I think "the important point here" is that a command
that behaves differently when the region is active should
NOT act on the region when `transient-mark-mode' is off.
Why?  Because the notion of "active region" applies only
when `transient-mark-mode' is on.  Any special behavior
provided only when the region is active is, well, only
for when the region is active.  And that's never the case
when `transient-mark-mode' is on.

This was not the case for `sort-lines', as you point out.
It did NOT, and does not, behave differently when the
region is active from when it is inactive.

bug#13521: `sort-lines' on the empty region

[Eli Zaretskii]

> Date: Sat, 5 Sep 2020 08:22:58 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: stefan@marxist.se, xfq.free@gmail.com, rgm@gnu.org,
>         monnier@iro.umontreal.ca, 13521@debbugs.gnu.org
> 
> > > I doubt that someone who has `transient-mark-mode' off
> > > would ever want commands such as `flush-lines' and
> > > `sort-lines' to act on the region.  And if they did, I
> > > expect they'd just narrow to it.
> > 
> > Please don't doubt, and please don't impose unnecessary commands on
> > users who have transient-mark-mode off.
> 
> Please don't claim that I imposed any commands on anyone.

You expected them to narrow the buffer.  That takes additional
commands.

> The region is nearly always present and usually nonempty.
> A user with `transient-mark-mode' off would typically
> (IMHO) be bothered if `flush-lines' started always acting
> on the region (it would be almost always: whenever there's
> a mark in the buffer and the region is nonempty). 

So maybe flush-lines needs a separate solution, or none at all.  This
bug is about sort-lines, so flush-lines is a tangent.

> And I think "the important point here" is that a command
> that behaves differently when the region is active should
> NOT act on the region when `transient-mark-mode' is off.

That's not really relevant here.  We are talking about commands which
by virtue of their 'interactive' spec work on the region.  Such
commands shouldn't depend on the region being active.

> This was not the case for `sort-lines', as you point out.
> It did NOT, and does not, behave differently when the
> region is active from when it is inactive.

The proposed change would have made it behave differently, which is
why I've pointed that out.

bug#13521: `sort-lines' on the empty region

[Drew Adams]

> > > > I doubt that someone who has `transient-mark-mode' off
> > > > would ever want commands such as `flush-lines' and
> > > > `sort-lines' to act on the region.  And if they did, I
> > > > expect they'd just narrow to it.
> > >
> > > Please don't doubt, and please don't impose unnecessary commands on
> > > users who have transient-mark-mode off.
> >
> > Please don't claim that I imposed any commands on anyone.
> 
> You expected them to narrow the buffer.  That takes additional
> commands.

I do expect that, for `flush-lines', which is what I
was thinking of.  You correctly pointed out that
`sort-lines' is different.

> > The region is nearly always present and usually nonempty.
> > A user with `transient-mark-mode' off would typically
> > (IMHO) be bothered if `flush-lines' started always acting
> > on the region (it would be almost always: whenever there's
> > a mark in the buffer and the region is nonempty).
> 
> So maybe flush-lines needs a separate solution, or none at all.  This
> bug is about sort-lines, so flush-lines is a tangent.

I didn't introduce `flush-lines' into the thread.
But yes, the subject line says `sort-lines', and
`flush-lines' is a very different case.

> > And I think "the important point here" is that a command
> > that behaves differently when the region is active should
> > NOT act on the region when `transient-mark-mode' is off.
> 
> That's not really relevant here.  We are talking about commands which
> by virtue of their 'interactive' spec work on the region.  Such
> commands shouldn't depend on the region being active.

Yes, if this is only about commands like `sort-lines'.
Introducing `flush-lines' here was apparently a mistake,
which muddied the waters.

> > This was not the case for `sort-lines', as you point out.
> > It did NOT, and does not, behave differently when the
> > region is active from when it is inactive.
> 
> The proposed change would have made it behave differently, which is
> why I've pointed that out.

Good.  Agreed.