Skip to content

ls implement size display#2435

Closed
hbina wants to merge 8 commits intouutils:masterfrom
hbina:hbina-ls-implement-size-display
Closed

ls implement size display#2435
hbina wants to merge 8 commits intouutils:masterfrom
hbina:hbina-ls-implement-size-display

Conversation

@hbina
Copy link
Contributor

@hbina hbina commented Jun 19, 2021

Implements the flag -s or --size to display the size of file in blocks.
Depends on for the refactor #2436

@hbina hbina changed the title Hbina ls implement size display ls implement size display Jun 19, 2021
@sylvestre
Copy link
Contributor

Please add a test for this :)
thanks

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@hbina hbina force-pushed the hbina-ls-implement-size-display branch from d2b80e1 to 44d748d Compare June 19, 2021 08:18
@hbina
Copy link
Contributor Author

hbina commented Jun 19, 2021

Added some tests based on this

coreutils on  hbina-ls-refactor-options-module [!?] is 📦 v0.0.6 via 🦀 v1.53.0 took 3s 
❯ for i in {0..123456..1}; do echo "a"; done > number.txt && ls -s number.txt 
244 number.txt

coreutils on  hbina-ls-refactor-options-module [!?] is 📦 v0.0.6 via 🦀 v1.53.0 took 3s 
❯ for i in {0..234567..1}; do echo "a"; done > number.txt && ls -s number.txt 
460 number.txt

coreutils on  hbina-ls-refactor-options-module [!?] is 📦 v0.0.6 via 🦀 v1.53.0 took 7s 
❯ for i in {0..345678..1}; do echo "a"; done > number.txt && ls -s number.txt 
676 number.txt

coreutils on  hbina-ls-refactor-options-module [!?] is 📦 v0.0.6 via 🦀 v1.53.0 took 10s 
❯ for i in {0..456789..1}; do echo "a"; done > number.txt && ls -s number.txt 
896 number.txt

@sylvestre
Copy link
Contributor

This test fails:

---- test_ls::test_ls_size stdout ----
current_directory_resolved: 
touch: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpJKz62i\test-1
open(append): C:\Users\RUNNER~1\AppData\Local\Temp\.tmpJKz62i\test-1
touch: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpJKz62i\test-2
open(append): C:\Users\RUNNER~1\AppData\Local\Temp\.tmpJKz62i\test-2
touch: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpJKz62i\test-3
open(append): C:\Users\RUNNER~1\AppData\Local\Temp\.tmpJKz62i\test-3
touch: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpJKz62i\test-4
open(append): C:\Users\RUNNER~1\AppData\Local\Temp\.tmpJKz62i\test-4
run: D:\a\coreutils\coreutils\target\i686-pc-windows-gnu\debug\coreutils.exe ls -s
run: D:\a\coreutils\coreutils\target\i686-pc-windows-gnu\debug\coreutils.exe ls -s
thread 'test_ls::test_ls_size' panicked at 'assertion failed: `(left == right)`

Diff < left / right > :
<"246912 test-1  469134 test-2  691356 test-3  913578 test-4\n"
>"test-4  test-3  test-2  test-1\n"

', tests\common\util.rs:222:9

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@hbina hbina force-pushed the hbina-ls-implement-size-display branch from 8bfbbf2 to 897ec72 Compare June 19, 2021 10:44
hbina added 2 commits June 19, 2021 19:05
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>

Added tests for ls --size

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>

Fixed tests for windows

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@hbina hbina force-pushed the hbina-ls-implement-size-display branch from 897ec72 to 4039dbd Compare June 19, 2021 11:10
hbina added 2 commits June 19, 2021 19:16
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@sylvestre
Copy link
Contributor

Freebsd is failing on


---- test_ls::test_ls_size stdout ----
current_directory_resolved: 
touch: /tmp/.tmpc8bXmN/test-1
open(append): /tmp/.tmpc8bXmN/test-1
touch: /tmp/.tmpc8bXmN/test-2
open(append): /tmp/.tmpc8bXmN/test-2
touch: /tmp/.tmpc8bXmN/test-3
open(append): /tmp/.tmpc8bXmN/test-3
touch: /tmp/.tmpc8bXmN/test-4
open(append): /tmp/.tmpc8bXmN/test-4
run: /tmp/cirrus-ci-build/target/debug/coreutils ls -s
run: /tmp/cirrus-ci-build/target/debug/coreutils ls -s
thread 'test_ls::test_ls_size' panicked at 'assertion failed: `(left == right)`

�[1mDiff�[0m �[31m< left�[0m / �[32mright >�[0m :
�[31m<"244 test-1\n�[0m�[1;48;5;52;31m512�[0m�[31m test-2\n7�[0m�[1;48;5;52;31m3�[0m�[31m6 test-3\n�[0m�[1;48;5;52;31m92�[0m�[31m8 test-4\n"�[0m
�[32m>"244 test-1\n�[0m�[1;48;5;22;32m460�[0m�[32m test-2\n�[0m�[1;48;5;22;32m6�[0m�[32m76 test-3\n8�[0m�[1;48;5;22;32m96�[0m�[32m test-4\n"�[0m

', tests/common/util.rs:222:9


failures:
    test_ls::test_ls_size

https://github.com/uutils/coreutils/pull/2435/checks?check_run_id=2866873876
could you please fix that?

@hbina
Copy link
Contributor Author

hbina commented Jun 25, 2021

I have no idea how to use FreeBSD. I am trying tho.

@sylvestre
Copy link
Contributor

We have a few conflicts on it, sorry

@hbina
Copy link
Contributor Author

hbina commented Jul 5, 2021

Do you have any idea why the fixture doesn't work in FreeBSD, i tried my ls -la normally and it seems to work fine.

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@skyronic
Copy link

skyronic commented Jul 9, 2021

Hey @hbina

I'm new to rust and thought I'd implement ls -s before I saw that you've got a much more comprehensive implementation here!

Can I ask why you're calculating the block sizes instead of using md.blocks() for block sizes? I tried using it and it seemed to work fine in my basic implementation

Just curious about why you took this approach, so I can understand it better. Thanks!

@hbina
Copy link
Contributor Author

hbina commented Jul 9, 2021

I actually mixed in some refactoring as well. I think if your implementation works on Windows and BSD, it might be better. I don't think I am doing anything different than yours.

@hbina hbina closed this Jul 9, 2021
@skyronic
Copy link

skyronic commented Jul 9, 2021

I just realized my implementation doesn't even compile on windows 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants