Kostas Blekos <mplekos@physics.upatras.gr> has mailed me with a patch to ls(1) that allows sorting the output files by size: % orion:/d/src/bin/ls$ ./ls -lS ls* % -rw-rw-r-- 1 keramida keramida 3214 Mar 21 16:44 ls.h % -rw-rw-r-- 1 keramida keramida 6028 May 18 17:37 ls.1.gz % -rw-rw-r-- 1 keramida keramida 15564 May 18 17:37 ls.1 % -rw-rw-r-- 1 keramida keramida 21676 May 18 17:27 ls.c % -rw-rw-r-- 1 keramida keramida 25924 May 18 17:27 ls.o % -rwxrwxr-x 1 keramida keramida 57206 May 18 17:38 ls % orion:/d/src/bin/ls$ ./ls -rlS ls* % -rwxrwxr-x 1 keramida keramida 57206 May 18 17:38 ls % -rw-rw-r-- 1 keramida keramida 25924 May 18 17:27 ls.o % -rw-rw-r-- 1 keramida keramida 21676 May 18 17:27 ls.c % -rw-rw-r-- 1 keramida keramida 15564 May 18 17:37 ls.1 % -rw-rw-r-- 1 keramida keramida 6028 May 18 17:37 ls.1.gz % -rw-rw-r-- 1 keramida keramida 3214 Mar 21 16:44 ls.h % orion:/d/src/bin/ls$ 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: # ls -o size,mtime,name
Giorgos Keramidas <keramida@FreeBSD.org> wrote: > >Description: > > 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 @@ > > 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 = revstatcmp; > else /* Use modification time. */ > sortfcn = revmodcmp; > + if (f_sizesort) sortfcn = 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 = statcmp; > else /* Use modification time. */ > sortfcn = modcmp; > + if (f_sizesort) sortfcn = sizecmp; As above
On 2005-06-01 11:25, Dima Dorfman <dd@freebsd.org> wrote: > Giorgos Keramidas <keramida@FreeBSD.org> wrote: > > >Description: > > > > 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. Agreed. > > + > > +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).) I know. I just patched a local bin/ls and tried that it all works, so style changes like those you mentioned are perfectly fine :-)
State Changed From-To: open->closed Patch committed. Thanks!