bug#23794: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

bug#23794: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

[Robert Weiner]

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

sort-lines calls forward-line rather than forward-visible line, so if
you have emacs outline entries that are collapsed/hidden to single lines
each and you try to sort them, their bodies and subtrees are sorted
separately because forward-visible-line is not used.

This patch fixes this problem and also unifies the calling convention of
forward-visible-line with that of forward-line (allowing it to take an
optional argument) leading to a cleaner calling convention.

Please apply it as soon as you can as Hyperbole uses sort-lines to sort
its contact manager records and right now this doesn't work.  Although,
sort-subr could be called directly for this application, sort-lines
should work properly with both visible and invisible text and the patch
is quite simple.

Thanks,

Bob
--------

In GNU Emacs 25.0.94.1 (x86_64-apple-darwin13.4.0, NS appkit-1265.21
Version 10.9.5 (Build 13F1603))
 of 2016-05-17 built on builder10-9.local
Windowing system distributor 'Apple', version 10.3.1404
Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp''

Configured features:
NOTIFY ACL LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

----------------

*** sort-orig.el.gz 2016-06-18 11:31:13.000000000 -0400
--- sort.el.gz 2016-06-18 11:31:13.000000000 -0400
***************
*** 210,216 ****
        (goto-char (point-min))
        (let ;; To make `end-of-line' and etc. to ignore fields.
   ((inhibit-field-text-motion t))
! (sort-subr reverse 'forward-line 'end-of-line)))))

  ;;;###autoload
  (defun sort-paragraphs (reverse beg end)
--- 210,216 ----
        (goto-char (point-min))
        (let ;; To make `end-of-line' and etc. to ignore fields.
   ((inhibit-field-text-motion t))
! (sort-subr reverse 'forward-visible-line 'end-of-visible-line)))))

  ;;;###autoload
  (defun sort-paragraphs (reverse beg end)


*** simple-orig.el.gz 2016-06-18 11:29:58.000000000 -0400
--- simple.el.gz 2016-06-18 11:29:58.000000000 -0400
***************
*** 4909,4918 ****
  (kill-region (point)
       (progn (forward-visible-line arg) (point))))))

! (defun forward-visible-line (arg)
!   "Move forward by ARG lines, ignoring currently invisible newlines only.
  If ARG is negative, move backward -ARG lines.
  If ARG is zero, move to the beginning of the current line."
    (condition-case nil
        (if (> arg 0)
   (progn
--- 4909,4919 ----
  (kill-region (point)
       (progn (forward-visible-line arg) (point))))))

! (defun forward-visible-line (&optional arg)
!   "Move forward by optional ARG lines (default = 1), ignoring currently
invisible newlines only.
  If ARG is negative, move backward -ARG lines.
  If ARG is zero, move to the beginning of the current line."
+   (if (null arg) (setq arg 1))
    (condition-case nil
        (if (> arg 0)
   (progn

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

bug#23794: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

[Eli Zaretskii]

> From: Robert Weiner <rsw@gnu.org>
> Date: Sat, 18 Jun 2016 11:47:08 -0400
> 
> sort-lines calls forward-line rather than forward-visible line, so if
> you have emacs outline entries that are collapsed/hidden to single lines
> each and you try to sort them, their bodies and subtrees are sorted
> separately because forward-visible-line is not used.
> 
> This patch fixes this problem and also unifies the calling convention of
> forward-visible-line with that of forward-line (allowing it to take an
> optional argument) leading to a cleaner calling convention.
> 
> Please apply it as soon as you can as Hyperbole uses sort-lines to sort
> its contact manager records and right now this doesn't work.  Although,
> sort-subr could be called directly for this application, sort-lines
> should work properly with both visible and invisible text and the patch
> is quite simple.

I don't think we can make such a backward-incompatible change without
(a) an entry in NEWS, (b) suitable changes in the manual(s), and
(c) some way of getting back the old behavior (which could be by way
of having this new behavior as an optional one).

Thanks.

bug#23794: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

[Robert Weiner]

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

On Sat, Jun 18, 2016 at 1:26 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Robert Weiner <rsw@gnu.org>
> > Date: Sat, 18 Jun 2016 11:47:08 -0400
> >
> > sort-lines calls forward-line rather than forward-visible line, so if
> > you have emacs outline entries that are collapsed/hidden to single lines
> > each and you try to sort them, their bodies and subtrees are sorted
> > separately because forward-visible-line is not used.
>
> I don't think we can make such a backward-incompatible change without
> (a) an entry in NEWS,


I am willing to write this.


> (b) suitable changes in the manual(s), and
>

I looked at both the Emacs and the Elisp manuals and the only change
necessary would be in the elisp manual where the full code for sort-lines
is shown, so that would be a simple replace.

(c) some way of getting back the old behavior (which could be by way
> of having this new behavior as an optional one).
>

See below.

For clarity, the original behavior of sort-lines is what the patch
restores.  The backward-incompatibility to which you refer is then just an
implementation error that occurred when switching over to the overlay
implementation of outlines as there was never any documentation that I can
see that suggested any behavior change.  There certainly could be better
documentation as to whether a 'line' refers to a visible line, an invisible
line or both but many functions do not delineate this.  A major reason for
making lines invisible is so that they are not treated as regular lines
when functions are applied to buffer text.  Thus, sort-lines should by
default operate on visible lines.  It could be extended or another function
could be written to operate on invisible lines as well, e.g.
sort-invisible-lines and an alias could be made to sort-lines to be called
sort-visible-lines.  All of this in the future.  The only thing I am
suggesting for right now is to restore the original behavior.  Note that if
all lines are visible, the patch codes works as well.  The issue is that
when lines are invisible the current code in Emacs does not work in a very
useful way.

Bob

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

bug#23794: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

[Eli Zaretskii]

> From: Robert Weiner <rsw@gnu.org>
> Date: Sat, 18 Jun 2016 13:42:01 -0400
> Cc: 23794@debbugs.gnu.org
> 
>  I don't think we can make such a backward-incompatible change without
>  (a) an entry in NEWS,
> 
> I am willing to write this.

Thanks.

>  (b) suitable changes in the manual(s), and
> 
> I looked at both the Emacs and the Elisp manuals and the only change necessary would be in the elisp
> manual where the full code for sort-lines is shown, so that would be a simple replace.

I think the fact that sorting ignores invisible text should be
prominently mentioned in both the user manual and the ELisp manual,
where the sort functions and commands are described.

>  (c) some way of getting back the old behavior (which could be by way
>  of having this new behavior as an optional one).
> 
> See below. 
> 
> For clarity, the original behavior of sort-lines is what the patch restores. The backward-incompatibility to which
> you refer is then just an implementation error that occurred when switching over to the overlay implementation
> of outlines as there was never any documentation that I can see that suggested any behavior change. There
> certainly could be better documentation as to whether a 'line' refers to a visible line, an invisible line or both
> but many functions do not delineate this. A major reason for making lines invisible is so that they are not
> treated as regular lines when functions are applied to buffer text. Thus, sort-lines should by default operate on
> visible lines. It could be extended or another function could be written to operate on invisible lines as well, e.g.
> sort-invisible-lines and an alias could be made to sort-lines to be called sort-visible-lines. All of this in the
> future. The only thing I am suggesting for right now is to restore the original behavior. Note that if all lines are
> visible, the patch codes works as well. The issue is that when lines are invisible the current code in Emacs
> does not work in a very useful way.

I think I already responded to this argument in my previous message.

bug#23794: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

[Eli Zaretskii]

> Date: Sat, 18 Jun 2016 20:49:12 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 23794@debbugs.gnu.org
> 
> > For clarity, the original behavior of sort-lines is what the patch restores. The backward-incompatibility to which
> > you refer is then just an implementation error that occurred when switching over to the overlay implementation
> > of outlines as there was never any documentation that I can see that suggested any behavior change. There
> > certainly could be better documentation as to whether a 'line' refers to a visible line, an invisible line or both
> > but many functions do not delineate this. A major reason for making lines invisible is so that they are not
> > treated as regular lines when functions are applied to buffer text. Thus, sort-lines should by default operate on
> > visible lines. It could be extended or another function could be written to operate on invisible lines as well, e.g.
> > sort-invisible-lines and an alias could be made to sort-lines to be called sort-visible-lines. All of this in the
> > future. The only thing I am suggesting for right now is to restore the original behavior. Note that if all lines are
> > visible, the patch codes works as well. The issue is that when lines are invisible the current code in Emacs
> > does not work in a very useful way.
> 
> I think I already responded to this argument in my previous message.

In case it wasn't clear, I would welcome a change that is specific to
outline modes, whereby sorting would produce the same effect as it did
in Emacs 21 in these modes.  But even in outline modes, I think there
should be a way of getting the old behavior back.

bug#23794: Fwd: Fwd: bug#23789: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

[Robert Weiner]

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

---------- Forwarded message ----------
From: Richard Stallman <rms@gnu.org>
Date: Wed, Jun 22, 2016 at 2:34 AM
Subject: Re: Fwd: bug#23789: Emacs 25.0.94: Patch to make sort-lines
respect visible lines (fairly urgent)
To: rswgnu@gmail.com

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > I think the old behavior was only correct for Outline mode in SOME
  > > cases.  Not in all cases.  Likewise for the new behavior.

  > Can you list the cases you think exist and how you think each should
  > be handled?

When the first line of some substructure is visible and the rest of
that substructure is hidden, it seems to make sense to sort the
substructure with the first line.

However, if there are two substructures at the same level
and they are both invisible, it does not seem right for them
to stick together in sorting.

I have a feeling that sort-lines doesn't really make sense for an
outline, and other operations are needed for sorting in outlines.
But I am not sure of that.

--
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

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

bug#23794: Fwd: bug#23789: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

[Robert Weiner]

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

On Mon, Aug 28, 2017 at 12:00 AM, Robert Weiner <rsw@gnu.org> wrote:

> ---------- Forwarded message ----------
> From: Richard Stallman <rms@gnu.org>
> Date: Wed, Jun 22, 2016 at 2:34 AM
> Subject: Re: Fwd: bug#23789: Emacs 25.0.94: Patch to make sort-lines
> respect visible lines (fairly urgent)
> To: rswgnu@gmail.com
>
>   > > I think the old behavior was only correct for Outline mode in SOME
>   > > cases.  Not in all cases.  Likewise for the new behavior.
>
>   > Can you list the cases you think exist and how you think each should
>   > be handled?
>
> When the first line of some substructure is visible and the rest of
> that substructure is hidden, it seems to make sense to sort the
> substructure with the first line.
>

​I agree.
​

> ​​
> However, if there are two substructures at the same level
> ​​
> and they are both invisible, it does not seem right for them
> ​​
> to stick together in sorting.
>

​This I'll argue against.  The point of hiding the substructure in an
outline is not just a visual one but also so you can operate on a hierarchy
as a single entity.  You might want to move it, delete it or sort it but
you typically want to treat it as a single unit.  Its visual
representation, often as a single line, reflects this.

So, I would argue, when you sort an outline with certain elements visible
and others hidden, you want to sort just the visible elements and keep the
hidden substructures together with their visible ancestors but not sort
them.  If you want to sort substructure, you expand it and visually reflect
its existence.  That would make for pretty consistent and understandable
behavior.

​​
> I have a feeling that sort-lines doesn't really make
> ​​
> sense for an
> ​
> ​​
> o
> ​​
> u
> ​​
> t
> ​​
> l
> ​​
> i
> ​​
> n
> ​​
> e
> ​​
>
​​
​Personally, I have often found it useful.
​

> ​​
> , and other operations are needed for sortin
> ​​
> g in outlines.
>
​​
​Yes, we could write other commands or we could apply something like the
simple patch I provided and make this do something ​useful before Emacs 26
is released.
We all agree that the current behavior of sort-lines is not useful in
outlines as it stands.

​​
> But I am not sure of that.
> ​​
>
​​
​I have contributed what I can on this and I hope someone is willing to try
to get this patched before Emacs 26 goes out or we'll have a whole other
long-lived generation of Emacs without a basic way to sort visible lines in
a file.

Bob
​

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

bug#23794: Emacs 25.0.94: Patch to make sort-lines respect visible lines (fairly urgent)

[Lars Ingebrigtsen]

Robert Weiner <rsw@gnu.org> writes:

> sort-lines calls forward-line rather than forward-visible line, so if
> you have emacs outline entries that are collapsed/hidden to single lines
> each and you try to sort them, their bodies and subtrees are sorted
> separately because forward-visible-line is not used.
>
> This patch fixes this problem and also unifies the calling convention of
> forward-visible-line with that of forward-line (allowing it to take an
> optional argument) leading to a cleaner calling convention.

This was three years ago, and from what I can tell, nobody has changed
sort-lines to be aware of invisible lines...  I can see the case for
allowing the caller to control this, but none of the patches proposed
look acceptable to me (for instance, one made sort.el check for outline
mode explicitly, which seems backwards).

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