Date: Wed, 1 Jun 2005 11:30:07 GMT From: Dima Dorfman <dd@freebsd.org> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/81625: "sort by size" option for ls(1) Message-ID: <200506011130.j51BU7W8049701@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/81625; it has been noted by GNATS.
From: Dima Dorfman <dd@freebsd.org>
To: Giorgos Keramidas <keramida@freebsd.org>
Cc: mplekos@physics.upatras.gr, FreeBSD-gnats-submit@freebsd.org
Subject: Re: bin/81625: "sort by size" option for ls(1)
Date: Wed, 1 Jun 2005 11:25:28 +0000
--r5Pyd7+fXNt84Ff3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Giorgos Keramidas <keramida@FreeBSD.org> wrote:
> >Description:
>=20
> Kostas Blekos <mplekos@physics.upatras.gr> has mailed me with a patch
> to ls(1) that allows sorting the output files by size:
Cool, looks like a nice improvement. Some minor comments below. Part
of the man page update seems to be missing, and I want to change the
default polarity, but the rest are just minor style nits.
> The -S option is not exactly optimal, but there are so many option
> letters that are taken by existing ls(1) features that there isn't
> much of a choise, unless we modify ls to use something like -o for
> picking a selection order:
-S is okay. OpenBSD and NetBSD use it for the same thing, except that
their default is to sort the largest files first.
-S Sort by size, largest file first.
Can we change the patch to do that? I'm not sure why they picked
descending order, but compatibility would be good here.
> RCS file: /home/ncvs/src/bin/ls/cmp.c,v
> @@ -139,3 +139,19 @@
> =20
> return (statcmp(b, a));
> }
> +
> +int
> +sizecmp(const FTSENT *a, const FTSENT *b)
> +{
> + if (b->fts_statp->st_size > a->fts_statp->st_size)
> + return (-1);
Style nit: Please add a blank line above the if. If a function has no
local variables, there should still be a blank line after the brace.
(See the usage() example in style(9).)
> +int
> +revsizecmp(const FTSENT *a, const FTSENT *b)
> +{
> + return (sizecmp(b, a));
As above
> RCS file: /home/ncvs/src/bin/ls/ls.1,v
> @@ -133,6 +133,8 @@
> options.
> .It Fl R
> Recursively list subdirectories encountered.
> +.It Fl S
> +Sort by size (before sorting
This seems to have been cut off.
> @@ -405,6 +409,7 @@
> sortfcn =3D revstatcmp;
> else /* Use modification time. */
> sortfcn =3D revmodcmp;
> + if (f_sizesort) sortfcn =3D revsizecmp;
Is there a reason this condition can't be checked along with all the
others in the if/else if block? Also, the predominant style in this
file seems to be to put the body of the if on a new line below the
conditional.
> @@ -414,6 +419,7 @@
> sortfcn =3D statcmp;
> else /* Use modification time. */
> sortfcn =3D modcmp;
> + if (f_sizesort) sortfcn =3D sizecmp;
As above
--r5Pyd7+fXNt84Ff3
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
iD8DBQFCnZsnBzAFW2n65YIRAvekAJ0VeMsDPuxLns3kF4KhpKHWDucYngCdHPH6
A8lH3moz/8NgF0JLjATjCOY=
=72vr
-----END PGP SIGNATURE-----
--r5Pyd7+fXNt84Ff3--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200506011130.j51BU7W8049701>