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: Improve the access to metadata of the files #5660

Merged
merged 12 commits into from
Dec 25, 2023

Conversation

sylvestre
Copy link
Sponsor Contributor

Should fix tests/ls/stat-free-color.sh

@sylvestre
Copy link
Sponsor Contributor Author

Do not merge. I need to simplify the color_name function

@sylvestre sylvestre force-pushed the stat-free-color branch 2 times, most recently from 04c40a8 to 162afc0 Compare December 17, 2023 09:59
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/ls/stat-free-color is no longer failing!

src/uu/ls/src/ls.rs Outdated Show resolved Hide resolved
ls_colors: &LsColors,
style_manager: &mut StyleManager,
out: &mut BufWriter<Stdout>,
check_for_deref: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_for_deref seems unnecessary to me because its value is linked to target_symlink: it's true if target_symlink is Some(..), and false, if target_symlink is None. And as color_name is only called twice, it might make sense to split the function into two functions.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wahou, well spotted, bravo :)

src/uu/ls/src/ls.rs Outdated Show resolved Hide resolved
@uutils uutils deleted a comment from github-actions bot Dec 20, 2023
@uutils uutils deleted a comment from github-actions bot Dec 20, 2023
@uutils uutils deleted a comment from github-actions bot Dec 20, 2023
@cakebaker cakebaker merged commit 6475e6f into uutils:main Dec 25, 2023
57 of 59 checks passed
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.

2 participants