ls: -k is not implemented #2257

sylvestre · 2021-05-22T16:57:54Z

With GNU:

$ ls -ks README.md
20 README.md

us:

$ ls -ks README.md
error: Found argument '-k' which wasn't expected, or isn't valid in this context

man:

       -k, --kibibytes
              default  to  1024-byte  blocks for disk usage; used only with -s
              and per directory totals

The text was updated successfully, but these errors were encountered:

siebenHeaven · 2021-05-23T06:40:05Z

Note the prerequisite -s is not implemented as well:

$ ls -s README.md
20 README.md

uutils ls:

target/debug/coreutils ls -s README.md
error: Found argument '-s' which wasn't expected, or isn't valid in this context

USAGE:
    ls [OPTION]... [FILE]...

For more information try --help

TheAlakazam · 2021-06-04T18:30:06Z

Hi, can I work on implementing this?

sylvestre · 2021-06-04T18:34:34Z

sure

hbina · 2021-06-13T04:10:10Z

If anyone wants to pick this up. I suggest improving the function display_item_long here to be configurable. Where:

cargo run -- ls --format=long is equivalent to everything being enabled.
cargo run -- ls -s only have the size display to be enabled.

This way the function can be reused for future purposes.

I might come back for this later but for I need to do something else for now.

Cheers!

aburn · 2021-07-31T20:46:40Z

Hi,

I think I have the basic changes ready for the -s & -k options. I would like add few tests to get some coverage. so far I've been doing manual verification if the output matches what ls on my system (OS X) gives me. Code can be found @ https://github.com/aburn/coreutils .

Yet to fix:

display total size. For the ls output on my system the behavior seems to give the output in block EVEN if -h(Human readable) option is specified. I did NOT make this change. Still deciding if we want to keep that behavior or deviate from this and keep displaying the total in human readable format when -s option is specified.
Add tests. Any suggestions on what to cover.
- I will add tests for failure when reading from the environment variable BLOCKSIZE. There can be parsing issues etc when the variable is set. I've tested this manually but would help if there is some automated test for that case.
- would like some suggestions on what else can be tested for this option.
Test other formatting option.
- I've tested comma formatting ONLY so far. Will see if there are any specific changes needed to formatting in other formats.
Test in other platforms (Windows / Linux) - I do NOT have any Windows machine, so might NOT be able to test it (hence the code is under #[cfg(unix)]). Will get to testing on Linux once done writing the testcases.

Other than these, any inputs/ suggestions are appreciated. This is my first time trying to contribute :). Hopefully I've covered all the details in this post here. I will raise a PR once basic I'm done writing / testing basic test cases.

Below captures few outputs:
Long format:

cargo run -- ls --format=long   
   Compiling uu_ls v0.0.7 (/Users/byammanu/rust/coreutils/src/uu/ls)
   Compiling coreutils v0.0.7 (/Users/byammanu/rust/coreutils)
    Finished dev [unoptimized + debuginfo] target(s) in 4.54s
     Running `target/debug/coreutils ls --format=long`
total 152
 16 -rw-r--r--  1 byammanu staff  5221 Jul 27 13:13 CODE_OF_CONDUCT.md
  8 -rw-r--r--  1 byammanu staff  4022 Jul 27 13:13 CONTRIBUTING.md
120 -rw-r--r--  1 byammanu staff 58503 Jul 27 13:13 Cargo.lock
 32 -rw-r--r--  1 byammanu staff 15208 Jul 27 13:13 Cargo.toml
  8 -rw-r--r--  1 byammanu staff  2296 Jul 27 13:13 DEVELOPER_INSTRUCTIONS.md
 16 -rw-r--r--  1 byammanu staff  7313 Jul 27 13:13 GNUmakefile
  8 -rw-r--r--  1 byammanu staff  1053 Jul 27 13:13 LICENSE
  8 -rw-r--r--  1 byammanu staff    55 Jul 27 13:13 Makefile
 24 -rw-r--r--  1 byammanu staff 10342 Jul 27 13:13 Makefile.toml
 40 -rw-r--r--  1 byammanu staff 18288 Jul 27 13:13 README.md
 16 -rw-r--r--  1 byammanu staff  7283 Jul 27 13:13 build.rs
  8 -rw-r--r--  1 byammanu staff    16 Jul 27 13:13 clippy.toml
  0 drwxr-xr-x 11 byammanu staff   352 Jul 27 13:13 docs
  0 drwxr-xr-x  4 byammanu staff   128 Jul 27 13:13 examples
  0 drwxr-xr-x  6 byammanu staff   192 Jul 27 13:13 src
  0 drwxr-xr-x  5 byammanu staff   160 Jul 27 13:15 target
  0 drwxr-xr-x  7 byammanu staff   224 Jul 27 13:13 tests
  0 drwxr-xr-x 15 byammanu staff   480 Jul 27 13:13 util

% BLOCKSIZE=2048 cargo run -- ls -sk
    Finished dev [unoptimized + debuginfo] target(s) in 0.22s
     Running `target/debug/coreutils ls -sk`
8 CODE_OF_CONDUCT.md  60 Cargo.lock  4 DEVELOPER_INSTRUCTIONS.md  4 LICENSE   12 Makefile.toml  8 build.rs     0 docs      0 src     0 tests
4 CONTRIBUTING.md     16 Cargo.toml  8 GNUmakefile                4 Makefile  20 README.md      4 clippy.toml  0 examples  0 target  0 util

% BLOCKSIZE=2048 cargo run -- ls -s 
    Finished dev [unoptimized + debuginfo] target(s) in 0.22s
     Running `target/debug/coreutils ls -s`
4 CODE_OF_CONDUCT.md  30 Cargo.lock  2 DEVELOPER_INSTRUCTIONS.md  2 LICENSE   6 Makefile.toml  4 build.rs     0 docs      0 src     0 tests
2 CONTRIBUTING.md     8 Cargo.toml   4 GNUmakefile                2 Makefile  10 README.md     2 clippy.toml  0 examples  0 target  0 util

% BLOCKSIZE=2048 cargo run -- ls -si
    Finished dev [unoptimized + debuginfo] target(s) in 0.23s
     Running `target/debug/coreutils ls -si`
363254251 4 CODE_OF_CONDUCT.md  363254254 8 Cargo.toml                 363254257 2 LICENSE        363254260 10 README.md   363254263 0 docs      363263688 0 target
363254252 2 CONTRIBUTING.md     363254255 2 DEVELOPER_INSTRUCTIONS.md  363254258 2 Makefile       363254261 4 build.rs     363254273 0 examples  363254935 0 tests
363254253 30 Cargo.lock         363254256 4 GNUmakefile                363254259 6 Makefile.toml  363254262 2 clippy.toml  363254276 0 src       363255561 0 util

% BLOCKSIZE=2048 cargo run -- ls -sik
    Finished dev [unoptimized + debuginfo] target(s) in 0.21s
     Running `target/debug/coreutils ls -sik`
363254251 8 CODE_OF_CONDUCT.md  363254254 16 Cargo.toml                363254257 4 LICENSE         363254260 20 README.md   363254263 0 docs      363263688 0 target
363254252 4 CONTRIBUTING.md     363254255 4 DEVELOPER_INSTRUCTIONS.md  363254258 4 Makefile        363254261 8 build.rs     363254273 0 examples  363254935 0 tests
363254253 60 Cargo.lock         363254256 8 GNUmakefile                363254259 12 Makefile.toml  363254262 4 clippy.toml  363254276 0 src       363255561 0 util

corresponding ls cmd ouputs:

% ls -sk
total 152
  8 CODE_OF_CONDUCT.md          4 DEVELOPER_INSTRUCTIONS.md  12 Makefile.toml               0 docs                        0 tests
  4 CONTRIBUTING.md             8 GNUmakefile                20 README.md                   0 examples                    0 util
 60 Cargo.lock                  4 LICENSE                     8 build.rs                    0 src
 16 Cargo.toml                  4 Makefile                    4 clippy.toml                 0 target

% ls -s  
total 304
 16 CODE_OF_CONDUCT.md          8 DEVELOPER_INSTRUCTIONS.md  24 Makefile.toml               0 docs                        0 tests
  8 CONTRIBUTING.md            16 GNUmakefile                40 README.md                   0 examples                    0 util
120 Cargo.lock                  8 LICENSE                    16 build.rs                    0 src
 32 Cargo.toml                  8 Makefile                    8 clippy.toml                 0 target
% ls -si
total 304
363254251  16 CODE_OF_CONDUCT.md        363254256  16 GNUmakefile               363254261  16 build.rs                  363263688   0 target
363254252   8 CONTRIBUTING.md           363254257   8 LICENSE                   363254262   8 clippy.toml               363254935   0 tests
363254253 120 Cargo.lock                363254258   8 Makefile                  363254263   0 docs                      363255561   0 util
363254254  32 Cargo.toml                363254259  24 Makefile.toml             363254273   0 examples
363254255   8 DEVELOPER_INSTRUCTIONS.md 363254260  40 README.md                 363254276   0 src

% ls -sik
total 152
363254251   8 CODE_OF_CONDUCT.md        363254256   8 GNUmakefile               363254261   8 build.rs                  363263688   0 target
363254252   4 CONTRIBUTING.md           363254257   4 LICENSE                   363254262   4 clippy.toml               363254935   0 tests
363254253  60 Cargo.lock                363254258   4 Makefile                  363254263   0 docs                      363255561   0 util
363254254  16 Cargo.toml                363254259  12 Makefile.toml             363254273   0 examples
363254255   4 DEVELOPER_INSTRUCTIONS.md 363254260  20 README.md                 363254276   0 src

tertsdiepraam · 2021-07-31T23:08:58Z

Hi @aburn,

Awesome! Your results look very nice!

We follow GNU by default (without looking at the GNU code for licensing reasons), so I think it's best to stick with what GNU is doing for the total size. The ls on mac is a little bit different, so I would recommend installing the GNU version and testing against that (see here for instance).

The tests don't need to be perfect on the first run. You can just open a PR (without #[cfg(unix)]) and let the CI run the tests on all platforms. We'll only have to run them locally on different operating systems when the CI uncovers some complicated issues (which I don't expect here).

You could also start with a draft PR. That makes it very easy for us to see the changes and give feedback.

Thanks for picking this up and just let me know if you have any more questions!

aburn · 2021-08-01T03:08:40Z

We follow GNU by default (without looking at the GNU code for licensing reasons), so I think it's best to stick with what GNU is doing for the total size. The ls on mac is a little bit different, so I would recommend installing the GNU version and testing against that (see here for instance).

Ok. I just checked on my Linux machine (hopefully its GNU coreutils installed by default) and the documentation is very different from the OS X version :( .

the default block size is different (seems to be 1024 on my Linux machine)
on my Linux machine, block size can be provided in human readable format (1K instead of 1024 etc)

Let me go through the GNU documentation and update my code accordingly.

The tests don't need to be perfect on the first run. You can just open a PR (without #[cfg(unix)]) and let the CI run the tests on all platforms. We'll only have to run them locally on different operating systems when the CI uncovers some complicated issues (which I don't expect here).

Thanks! thats good to know. Once I make the current code compatible with GNU, I will remove #[cfg(unix)].

You could also start with a draft PR. That makes it very easy for us to see the changes and give feedback.

This is a good idea. I will raise a draft PR with my current code as is to get some early feedback if I can. of-course, in parallel , I will also be making changes to align with GNU instead of what I have on OS X.

Thanks for the pointers @tertsdiepraam

sylvestre · 2022-01-30T09:02:34Z

If someone is interested, you can resurrect:
#2542

hbina · 2022-01-31T09:14:22Z

Let me try to necro this PR.

sylvestre · 2022-04-19T08:15:29Z

Seems that #3052 fixed it

sylvestre added the good first issue For newcomers! label May 22, 2021

skyronic mentioned this issue Jul 9, 2021

Add block size display to ls (ls -l) #2488

Closed

aburn mentioned this issue Aug 1, 2021

ls: provide -s & -k option #2542

Closed

tertsdiepraam added the U - ls label Jan 19, 2022

hbina mentioned this issue Feb 1, 2022

ls: Implement -s and -k #3015

Closed

sylvestre closed this as completed Apr 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ls: -k is not implemented #2257

ls: -k is not implemented #2257

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ls: -k is not implemented #2257

ls: -k is not implemented #2257

Comments

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!