GNU bug report logs - #31800
suggestion of improvement for sort-numeric-fields function.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31800 in the body.
You can then email your comments to 31800 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: SK Kim <tttuuu888 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: suggestion of improvement for sort-numeric-fields function.
Date: Wed, 13 Jun 2018 01:09:17 +0900

[Message part 1 (text/plain, inline)]

This is not likely a bug but sort-numeric-fields function does not allow
region with blank lines, while sort-lines does.

This was because sort-skip-fields occurs error with blank line. And when I
added condition for sort-skip-fields like below, sort-numeric-fields was
working with blank lines too.

                     (when (not (string-match-p "^\\s-*$" (thing-at-point
'line)))
                       (sort-skip-fields field))

So, I just hope sort-numeric-fields would work for region with blank lines
too.

I tested with GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.18.9) of 2018-05-29


Always appreciate for contribution.
Thanks.

[Message part 2 (text/html, inline)]

Message #8 received at 31800 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: SK Kim <tttuuu888 <at> gmail.com>
Cc: 31800 <at> debbugs.gnu.org
Subject: Re: bug#31800: suggestion of improvement for sort-numeric-fields
 function.
Date: Sat, 13 Jul 2019 05:41:56 +0200

SK Kim <tttuuu888 <at> gmail.com> writes:

> This is not likely a bug but sort-numeric-fields function does not
> allow region with blank lines, while sort-lines does.
>
> This was because sort-skip-fields occurs error with blank line. And
> when I added condition for sort-skip-fields like below,
> sort-numeric-fields was working with blank lines too.
>
>                      (when (not (string-match-p "^\\s-*$" (thing-at-point 'line)))
>                        (sort-skip-fields field))
>
> So, I just hope sort-numeric-fields would work for region with blank
> lines too.

Can you create a patch for this fix?

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

Message #11 received at 31800 <at> debbugs.gnu.org (full text, mbox):

From: SK Kim <tttuuu888 <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 31800 <at> debbugs.gnu.org
Subject: Re: bug#31800: suggestion of improvement for sort-numeric-fields
 function.
Date: Sat, 13 Jul 2019 17:21:18 +0900

[Message part 1 (text/plain, inline)]

I made this patch from github emacs-mirror repository.
I hope this will be of help.

Thanks.

2019년 7월 13일 (토) 오후 12:42, Lars Ingebrigtsen <larsi <at> gnus.org>님이 작성:

> SK Kim <tttuuu888 <at> gmail.com> writes:
>
> > This is not likely a bug but sort-numeric-fields function does not
> > allow region with blank lines, while sort-lines does.
> >
> > This was because sort-skip-fields occurs error with blank line. And
> > when I added condition for sort-skip-fields like below,
> > sort-numeric-fields was working with blank lines too.
> >
> >                      (when (not (string-match-p "^\\s-*$"
> (thing-at-point 'line)))
> >                        (sort-skip-fields field))
> >
> > So, I just hope sort-numeric-fields would work for region with blank
> > lines too.
>
> Can you create a patch for this fix?
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[Message part 2 (text/html, inline)]

[0001-Improve-sort-numeric-fields.patch (text/x-patch, attachment)]

Message #16 received at 31800 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: SK Kim <tttuuu888 <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 31800 <at> debbugs.gnu.org
Subject: Re: bug#31800: suggestion of improvement for sort-numeric-fields
 function.
Date: Mon, 20 Jan 2020 20:55:04 +0100

SK Kim <tttuuu888 <at> gmail.com> writes:

> I made this patch from github emacs-mirror repository.
> I hope this will be of help.
>
> Thanks.
>
> 2019년 7월 13일 (토) 오후 12:42, Lars Ingebrigtsen <larsi <at> gnus.org>님이 작성:
>
>  SK Kim <tttuuu888 <at> gmail.com> writes:
>
>  > This is not likely a bug but sort-numeric-fields function does not
>  > allow region with blank lines, while sort-lines does.
>  >
>  > This was because sort-skip-fields occurs error with blank line. And
>  > when I added condition for sort-skip-fields like below,
>  > sort-numeric-fields was working with blank lines too.
>  >
>  >                      (when (not (string-match-p "^\\s-*$" (thing-at-point 'line)))
>  >                        (sort-skip-fields field))
>  >
>  > So, I just hope sort-numeric-fields would work for region with blank
>  > lines too.

The patch below looks good to me.  Does anyone else have an opinion
here, or should I go ahead and commit it to master?

Best regards,
Stefan Kangas

> From d08c5d368337ee49bb72d9915c409edcbc73b4e0 Mon Sep 17 00:00:00 2001
> From: SeungKi Kim <tttuuu888 <at> gmail.com>
> Date: Sat, 13 Jul 2019 17:12:46 +0900
> Subject: [PATCH] Improve `sort-numeric-fields'
>
> * lisp/sort.el (sort-numeric-fields) : Allow including empty lines.
> ---
>  lisp/sort.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/sort.el b/lisp/sort.el
> index 6ea1c44060..3e9413e4af 100644
> --- a/lisp/sort.el
> +++ b/lisp/sort.el
> @@ -281,7 +281,8 @@ FIELD, BEG and END.  BEG and END specify region to sort."
>        ((inhibit-field-text-motion t))
>      (sort-fields-1 field beg end
>  		   (lambda ()
> -		     (sort-skip-fields field)
> +                     (unless (string-match-p "^\\s-*$" (thing-at-point 'line))
> +                       (sort-skip-fields field))
>  		     (let* ((case-fold-search t)
>  			    (base
>  			     (if (looking-at "\\(0x\\)[0-9a-f]\\|\\(0\\)[0-7]")

Message #19 received at 31800 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: SK Kim <tttuuu888 <at> gmail.com>, 31800 <at> debbugs.gnu.org
Subject: Re: bug#31800: suggestion of improvement for sort-numeric-fields
 function.
Date: Wed, 22 Jan 2020 14:44:36 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> The patch below looks good to me.  Does anyone else have an opinion
> here, or should I go ahead and commit it to master?

[...]

>> -		     (sort-skip-fields field)
>> +                     (unless (string-match-p "^\\s-*$" (thing-at-point 'line))
>> +                       (sort-skip-fields field))

I don't think this patch makes sense as is -- I think that's a
convoluted way of saying `looking-at'?  But it makes conceptual sense, I
think.

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

Message #22 received at 31800 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: SK Kim <tttuuu888 <at> gmail.com>, 31800 <at> debbugs.gnu.org
Subject: Re: bug#31800: suggestion of improvement for sort-numeric-fields
 function.
Date: Thu, 23 Jan 2020 15:50:12 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>>> -		     (sort-skip-fields field)
>>> +                     (unless (string-match-p "^\\s-*$" (thing-at-point 'line))
>>> +                       (sort-skip-fields field))
>
> I don't think this patch makes sense as is -- I think that's a
> convoluted way of saying `looking-at'?  But it makes conceptual sense, I
> think.

I'm not sure.  Do you mean to use:

    (looking-at "\\s*$")

Are we sure point is at the beginning of line here though?  The above
code doesn't require that, but also seems to be wrong if region is in
the middle of a line.  But I'm not sure how people typically use this
in that case.

BTW, shouldn't we also fix the same bug in sort-fields while we're at
it?

Best regards,
Stefan Kangas

Message #25 received at 31800 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: SK Kim <tttuuu888 <at> gmail.com>, 31800 <at> debbugs.gnu.org
Subject: Re: bug#31800: suggestion of improvement for sort-numeric-fields
 function.
Date: Fri, 24 Jan 2020 16:30:10 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>>>> -		     (sort-skip-fields field)
>>>> + (unless (string-match-p "^\\s-*$" (thing-at-point 'line))
>>>> +                       (sort-skip-fields field))
>>
>> I don't think this patch makes sense as is -- I think that's a
>> convoluted way of saying `looking-at'?  But it makes conceptual sense, I
>> think.
>
> I'm not sure.  Do you mean to use:
>
>     (looking-at "\\s*$")
>
> Are we sure point is at the beginning of line here though?  The above
> code doesn't require that, but also seems to be wrong if region is in
> the middle of a line.  But I'm not sure how people typically use this
> in that case.

I assumed that

  (thing-at-point 'line)

returns the current line, but I haven't actually looked at the code, but
you're right -- we don't know where point is, so that has to be moved
before checking.  In any case, using thing-at-point is not the right
thing.

> BTW, shouldn't we also fix the same bug in sort-fields while we're at
> it?

Yes.

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

Message #30 received at 31800 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: SK Kim <tttuuu888 <at> gmail.com>
Cc: 31800 <at> debbugs.gnu.org
Subject: Re: bug#31800: suggestion of improvement for sort-numeric-fields
 function.
Date: Thu, 03 Feb 2022 20:34:17 +0100

SK Kim <tttuuu888 <at> gmail.com> writes:

> This is not likely a bug but sort-numeric-fields function does not allow region with
> blank lines, while sort-lines does.
>
> This was because sort-skip-fields occurs error with blank line. And
> when I added condition for sort-skip-fields like below,
> sort-numeric-fields was working with blank lines too.

I've now fixed this in Emacs 29.

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.