bin/81625: "sort by size" option for ls(1)

[Dima Dorfman]

The following reply was made to PR bin/81625; it has been noted by GNATS.

From: Dima Dorfman <dd at freebsd.org>
To: Giorgos Keramidas <keramida at freebsd.org>
Cc: mplekos at physics.upatras.gr, FreeBSD-gnats-submit at 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 at FreeBSD.org> wrote:
 > >Description:
 >=20
 > Kostas Blekos <mplekos at 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--