coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] ls: add --sort=width (-W) option to sort by filename width


From: Jim Meyering
Subject: Re: [PATCH] ls: add --sort=width (-W) option to sort by filename width
Date: Sun, 11 Apr 2021 09:10:46 -0700

On Sat, Apr 10, 2021 at 8:22 PM Pádraig Brady <P@draigbrady.com> wrote:
> On 09/04/2021 23:51, Pádraig Brady wrote:
> > On 09/04/2021 13:02, Carl Edquist wrote:
> >> Dear Coreutils Maintainers,
> >>
> >> I'd like to introduce my favorite 'ls' option, '-W', which I have been
> >> enjoying using regularly over the last few years.
> >>
> >> The concept is just to sort filenames by their printed widths.
> >>
> >>
> >> (If this sounds odd, I invite you hear it out, try and see for yourself!)
> >>
> >>
> >> I am including a patch with my implementation and accompanying tests - as
> >> well as some sample output.  And I'll happily field any requests for
> >> improvements.
> >
> > I quite like this. It seems useful.
> > Also doing outside of ls is quite awkward,
> > especially considering multi column output.
> >
> > I would avoid the -W short option, as that would clash with ls from FreeBSD 
> > for example.
> > It's probably best to not provide a short option for this at all.
>
> Playing around with this a bit more,
> I really like how much more concise it makes output in general.
>
> I've attached two patches which I'll apply tomorrow at some stage.
>
> The first adjusts your patch to:
>    Remove -W short option.
>    Fix crash on systems with statx().
>    s/filename/file name/.
>    Expand in texinfo that --sort=width is most useful with -C (the default)
>
> The second is a performance improvement,
> as we're now calling quote_name_width a lot more.

Nice. I like this new option, too! Who would have thought :-)

Two minor stylistic suggestions:

move this declaration of "i" down into the loop where it's used, to
save two lines:

+static void
+update_current_files_cache (void)
+{
+  size_t i;
+
+  for (i = 0; i < cwd_n_used; i++)

And please use "char const *" (not const char *) in the added code of
ls.c. Admittedly this is a really small nit, especially since the
existing code in that file is not yet self-consistent. But at least
the preferred spelling outnumbers the others by about 3 to 1.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]