Re: [PATCH] ls: Clarify the effect of option -k

[Jean Delvare]

Hi Pádraig,

On Sat, 2 Dec 2017 16:37:02 -0800, Pádraig Brady wrote:
> On 01/12/17 02:26, Jean Delvare wrote:
> > Users may expect a different effect from option -k than is actually
> > implemented, especially when the effect of that option has changed
> > over time. The info page explains it well, but "ls --help" (and thus
> > the ls.1 man page) do not.
> > 
> > * src/ls.c: Improve the description of option -k.
> > ---
> >  src/ls.c |   10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > --- coreutils.orig/src/ls.c 2017-12-01 10:14:46.297551197 +0100
> > +++ coreutils/src/ls.c      2017-12-01 10:45:17.298050350 +0100
> > @@ -5203,7 +5203,15 @@ Sort entries alphabetically if none of -
> >    -i, --inode                print the index number of each file\n\
> >    -I, --ignore=PATTERN       do not list implied entries matching shell 
> > PATTERN\
> >  \n\
> > -  -k, --kibibytes            default to 1024-byte blocks for disk usage\n\
> > +"), stdout);
> > +      fputs (_("\
> > +  -k, --kibibytes            default to 1024-byte blocks for disk usage;\n\
> > +                               affects the per-directory block count 
> > written by\
> > +\n\
> > +                               -l and -s, and the size written by -s, but 
> > not\
> > +\n\
> > +                               the file size written by -l (use\n\
> > +                               '--block-size=1024' for that)\n\
> >  "), stdout);
> >        fputs (_("\
> >    -l                         use a long listing format\n\
> > 
> >   
> 
> 
> Way too verbose.

And you didn't see my first attempt, which was even longer ;-) I
expected some conciseness was desirable.

That being said... "too verbose" according to what? The description of
--time-style is longer that than, and those of -c, --time and
--quoting-style come close.

> How about:
> 
> diff --git a/src/ls.c b/src/ls.c
> index 073e135..35367a7 100644
> --- a/src/ls.c
> +++ b/src/ls.c
> @@ -5203,7 +5203,9 @@ Sort entries alphabetically if none of -cftuvSUX nor 
> --sort is specified.\n\
>    -i, --inode                print the index number of each file\n\
>    -I, --ignore=PATTERN       do not list implied entries matching shell 
> PATTERN\
>  \n\
> -  -k, --kibibytes            default to 1024-byte blocks for disk usage\n\
> +"), stdout);
> +      fputs (_("\
> +  -k, --kibibytes            default to 1024B blocks for -s and dir 
> 'total's\n\
>  "), stdout);
>        fputs (_("\
>    -l                         use a long listing format\n\
> 

This is super short, but I'm worried it no longer addresses the
problem. In fact I find it even more confusing than the original.
"dir 'total's", what is that? Looks like you are documenting it for
people who already know what it is doing, which isn't really useful.

A little bit of context: I am proposing this manual page update after a
customer of mine reported "ls -k" as being bugged and called it a
regression. They seemed right, until I read the description of the
option in the info page, and found the commit which changed the
behavior of the option. But I do not expect all users to do the same.
Most users will try "man ls" and "ls --help", they have no idea about
"info" and digging through the git log is not expected from simple
users. I know that the change is described in NEWS but this file is
huge, and documents the changes of all of coreutils. It can be hard to
find what you are looking for.

So I believe that the ls man page is the right place to explain the
intended purpose of option -k. The description of this option is twice
as long in the info page as my proposal is; you can see I already made
a decent effort to go to the essential. This option _is_ tricky,
otherwise its description in the info page would not be so long. So I
can't see anything wrong with some verbosity about it in the man page
as well.

Also you dropped the pointer to option --block-size, while I believe it
is particularly relevant due to the historical behavior of -k. -k used
to imply --block-size=1024, and most users used the former as a
shortcut for the latter. It no longer does, so it seems fair to provide
a suitable replacement.

Can we compromise on the following?

default to 1024-byte blocks for disk usage (-s and per-directory total
of -l); does not affect the file size of -l (use '--block-size=1024')

Thanks,
-- 
Jean Delvare
SUSE L3 Support