Re: sort --version-sort

[Jim Meyering]

Bruce Korb <address@hidden> wrote:
> diff --git a/ChangeLog-2008 b/ChangeLog-2008
> index aac9feb..da33f93 100644
> --- a/ChangeLog-2008
> +++ b/ChangeLog-2008
> @@ -1,3 +1,10 @@
> +2008-07-05  Bruce Korb  <address@hidden>
> +
> +     * src/sort.c: implement version number sort
> +     (compare_version): new procedure to do it.
> +     * tests/misc/sort-version: new test file
> +     * tests/Makefile.am: add it to the list

Thanks for the follow-up!
Note that in coreutils I'm not using a ChangeLog file
per se anymore.  Instead, it's generated from commit logs,
as mentioned in HACKING.  Speaking of ChangeLogs, I prefer
more (some would say "excruciating" ;-) detail, so rewrote that one:

commit 4c9fae4e97d95a9f89d1399a8aeb03051f0fec96
Author: Bruce Korb <address@hidden>
Date:   Thu Aug 14 06:24:59 2008 -0700

    sort: new option, --sort=version, for version number ordering

    * src/sort.c [struct keyfield] (version): New member.
    (usage): Describe --version-sort.
    (sort_options): Add 'V'.
    (long_options): Add "version-sort".
    (CHECK_TABLE, _ct_, SORT_TABLE, _st_): Define new macros.
    (check_args, sort_args, sort_types): Use these new macros in declarations.
    (ARGMATCH_VERIFY): Remove use.  No longer needed.
    (compare_version): New function.
    (key_compare): Add a case.
    (check_ordering_compatibility): Handle new type.
    (main): Likewise.  Reformat two expressions for readability.
    * tests/misc/sort-version: new test file
    * tests/Makefile.am: add it to the list
    * doc/coreutils.texi (sort invocation): Document it.
    * NEWS: Mention the new feature.

Note that you can generate a good first-cut template for the log
using Ralf Wildenhues' vc-chlog script, from the vc-dwim package:
http://www.gnu.org/software/vc-dwim/

A minor qualm:
  - This change adds a short option: -V
    Does any other vendor's sort program accept -V?

I like your cpp-based table-manipulation idiom that lets
us eliminate the use of ARGMATCH_VERIFY.  Thanks!

Also, I've made the following adjustments that will
be combined into your commit before I push.
Since the commit has your name on it, I'll wait to hear
back from you before doing this:

>From 929479026b3fd5910c165c22e1fae9f02e773342 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 15 Aug 2008 09:48:43 +0200
Subject: [PATCH] adjust

---
 ChangeLog-2008          |    7 -------
 NEWS                    |    4 ++--
 src/sort.c              |   20 +++++++++-----------
 tests/misc/sort-version |   36 +++++++++++++++++++++++-------------
 4 files changed, 34 insertions(+), 33 deletions(-)
 mode change 100644 => 100755 tests/misc/sort-version

diff --git a/ChangeLog-2008 b/ChangeLog-2008
index da33f93..aac9feb 100644
--- a/ChangeLog-2008
+++ b/ChangeLog-2008
@@ -1,10 +1,3 @@
-2008-07-05  Bruce Korb  <address@hidden>
-
-       * src/sort.c: implement version number sort
-       (compare_version): new procedure to do it.
-       * tests/misc/sort-version: new test file
-       * tests/Makefile.am: add it to the list
-
 2008-02-07  Jim Meyering  <address@hidden>

        We *do* need two different version files.
diff --git a/NEWS b/NEWS
index 72c885c..4979dd5 100644
--- a/NEWS
+++ b/NEWS
@@ -39,8 +39,8 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   represents the maximum number of inputs that will be merged at once.
   When processing more than NMERGE inputs, sort uses temporary files.

-  sort accepts still another new option --version-sort, specifying that
-  ordering is to be based on strverscmp(3).
+  sort accepts a new option --version-sort (-V, --sort=version),
+  specifying that ordering is to be based on strverscmp(3).

 ** Bug fixes

diff --git a/src/sort.c b/src/sort.c
index 4e5fc84..a617517 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -1822,19 +1822,19 @@ compare_random (char *restrict texta, size_t lena,

 static int
 compare_version (char *restrict texta, size_t lena,
-                char *restrict textb, size_t lenb)
+                char *restrict textb, size_t lenb)
 {
   int diff;

-  /*
-   *  It is necessary to save the character after the end of the field.
-   *  "strverscmp" works with NUL terminated strings.  Our blocks of
-   *  text are not necessarily terminated with a NUL byte.
-   */
+  /* It is necessary to save the character after the end of the field.
+     "strverscmp" works with NUL terminated strings.  Our blocks of
+     text are not necessarily terminated with a NUL byte. */
   char sv_a = texta[lena];
   char sv_b = textb[lenb];

-  texta[lena] = textb[lenb] = '\0';
+  texta[lena] = '\0';
+  textb[lenb] = '\0';
+
   diff = strverscmp (texta, textb);

   texta[lena] = sv_a;
@@ -1882,10 +1882,8 @@ keycompare (const struct line *a, const struct line *b)
                  (texta, textb));
          *lima = savea, *limb = saveb;
        }
-
       else if (key->version)
-        diff = compare_version (texta, lena, textb, lenb);
-
+       diff = compare_version (texta, lena, textb, lenb);
       else if (key->month)
        diff = getmonth (texta, lena) - getmonth (textb, lenb);
       /* Sorting like this may become slow, so in a simple locale the user
@@ -2745,7 +2743,7 @@ check_ordering_compatibility (void)
              + key->version + !!key->ignore))
        || (key->random && key->translate))
       {
-        /* The following is too big, but guaranteed to be "big enough". */
+       /* The following is too big, but guaranteed to be "big enough". */
        char opts[sizeof short_options];
        char *p = opts;
        if (key->ignore == nondictionary)
diff --git a/tests/misc/sort-version b/tests/misc/sort-version
old mode 100644
new mode 100755
index a4ebd40..3208f2e
--- a/tests/misc/sort-version
+++ b/tests/misc/sort-version
@@ -1,18 +1,29 @@
-#!/usr/bin/echo do-not-run-this-directly.-Use-a-shell
-# -*- Mode: shell-script -*-
+#!/bin/sh
+# exercise sort's --sort=version option
+
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.

 if test "$VERBOSE" = yes; then
   set -x
   sort --version
 fi

-. $top_srcdir/tests/test-lib.sh
-
-s_file=sort-ver-src
-g_file=sort-ver-good
-r_file=sort-ver-res
+. $srcdir/test-lib.sh

-cat > $s_file <<- _EOF_
+cat > in <<- _EOF_
        string start 5.0.0 end of str
        string start 5.00.0 end of str
        string start 5.1.0 end of str
@@ -35,8 +46,7 @@ cat > $s_file <<- _EOF_
        string start 5.90.0 end of str
        _EOF_

-
-cat > $g_file <<- _EOF_
+cat > exp <<- _EOF_
        string start 5.00.0 end of str
        string start 5.0.0 end of str
        string start 5.1.0 end of str
@@ -60,6 +70,6 @@ cat > $g_file <<- _EOF_
        _EOF_

 fail=0
-sort --sort=version -o $r_file $s_file
-compare $g_file $r_file >/dev/null 2>&1 || fail=1
-(exit $fail) ; exit $fail
+sort --sort=version -o out in || fail=1
+compare exp out || fail=1
+(exit $fail); exit $fail
--
1.6.0.rc3.9.g80b29