Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ls: Remove unnecessary trailing space when using the comma format (-m) #3615

Merged
merged 2 commits into from
Jun 14, 2022

Conversation

anastygnome
Copy link
Contributor

unnecessary trailing space was being added. because we were padding for alignment,
which is not required with -m

fixes #3608

Signed-off-by: anastygnome noreplygitemail@protonmail.com

unnecessary trailing space was being added. because we were padding for alignment,
which is not required with -m

fixes uutils#3608

Signed-off-by: anastygnome <noreplygitemail@protonmail.com>
@sylvestre
Copy link
Sponsor Contributor

Please add a test :)

@anastygnome anastygnome changed the title Remove unnecessary trailing space when using the comma format (-m) ls: Remove unnecessary trailing space when using the comma format (-m) Jun 11, 2022
@anastygnome
Copy link
Contributor Author

anastygnome commented Jun 11, 2022

Please add a test :)

Hey, excuse me, I don't know how to.

Sorry about that...
@sylvestre

@tertsdiepraam
Copy link
Member

tertsdiepraam commented Jun 11, 2022

You can just add a new test function to tests/by-util/test_ls.rs. There's already a lot of examples there that you can adapt to test this change.

@sylvestre
Copy link
Sponsor Contributor

cargo test --features unix test_ls will run all the ls tests

@anastygnome
Copy link
Contributor Author

You can just add a new test function to tests/by-util/test_ls.rs. There's already a lot of examples there that you can adapt to test this change.

I mean I don't know how to do the thing we did with seq 2000 > file in the issue.

@tertsdiepraam
Copy link
Member

Ah I see, I think it's best to do this in Rust code for the test. Something like this should work:

let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
at.touch("some-file");
at.append("some-file", (0..2000).map(|x| x.to_string()).collect::<Vec<_>>().join("\n"))

Signed-off-by: anastygnome <noreplygitemail@protonmail.com>
@anastygnome
Copy link
Contributor Author

@sylvestre the failure is due to tail notify tests. This is good to merge, the trailing test is working fine

@sylvestre
Copy link
Sponsor Contributor

sylvestre commented Jun 14, 2022

Bravo

Congrats! The gnu test tests/ls/m-option is no longer failing!

@sylvestre sylvestre merged commit 045c047 into uutils:main Jun 14, 2022
@anastygnome anastygnome deleted the fork branch June 14, 2022 17:05
@anastygnome anastygnome restored the fork branch June 14, 2022 17:10
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.

ls -sm is adding an unnecessary trailing space
3 participants