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

[Jim Meyering]

On Sun, Apr 11, 2021 at 9:10 AM Jim Meyering <jim@meyering.net> wrote:
>
> 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.

Oops. I see that you've pushed already.
I'll adjust if you don't beat me to it again. Thanks!