Skip to content

ls: Match the gnu behavior for colors#5603

Merged
sylvestre merged 4 commits intouutils:mainfrom
sylvestre:gnu-legacy
Dec 9, 2023
Merged

ls: Match the gnu behavior for colors#5603
sylvestre merged 4 commits intouutils:mainfrom
sylvestre:gnu-legacy

Conversation

@sylvestre
Copy link
Copy Markdown
Contributor

@sylvestre sylvestre commented Nov 30, 2023

No description provided.

@sylvestre
Copy link
Copy Markdown
Contributor Author

sylvestre commented Nov 30, 2023

it is failing but I would like to get the results

@sylvestre sylvestre changed the title Gnu legacy ls: Match the gnu behavior for colors Nov 30, 2023
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 30, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/ls/root-rel-symlink-color is no longer failing!
GNU test failed: tests/tail/inotify-dir-recreate. tests/tail/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

1 similar comment
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 30, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/ls/root-rel-symlink-color is no longer failing!
GNU test failed: tests/tail/inotify-dir-recreate. tests/tail/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

@sylvestre sylvestre marked this pull request as ready for review December 5, 2023 23:15
@sylvestre sylvestre requested review from cakebaker and tertsdiepraam and removed request for tertsdiepraam December 5, 2023 23:15
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 5, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/ls/root-rel-symlink-color is no longer failing!

cakebaker
cakebaker reviewed Dec 6, 2023
View reviewed changes
cakebaker
cakebaker reviewed Dec 6, 2023
View reviewed changes
src/uu/ls/Cargo.toml Outdated
terminal_size = { workspace = true }
glob = { workspace = true }
lscolors = { workspace = true }
lscolors = { workspace = true, features = ["gnu_legacy"] }
Copy link
Copy Markdown
Contributor

@cakebaker cakebaker Dec 6, 2023

Choose a reason for hiding this comment

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

Is this necessary? We didn't have the nu-ansi-term feature mentioned here.

cakebaker
cakebaker reviewed Dec 6, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@cakebaker cakebaker left a comment

Choose a reason for hiding this comment

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

I'm not sure whether this is the intended behavior or a bug. When running ls --color=always and redirecting the output to a file, the output of uutils ls and GNU ls differs. All but the first escape code are different, with the ones from uutils ls being prefixed with ^[[0m.

@sylvestre
Copy link
Copy Markdown
Contributor Author

sylvestre commented Dec 6, 2023

how do you compare the output ?
i did

ls > out
cat -v out

@cakebaker
Copy link
Copy Markdown
Contributor

cakebaker commented Dec 6, 2023

Yes, that's what I'm using.

@cakebaker
Copy link
Copy Markdown
Contributor

cakebaker commented Dec 6, 2023

Here the steps I did:

$ mkdir example example/a example/b
$ cargo run ls --color=always example/ > color.txt
$ ls --color=always example/ > colorgnu.txt
$ cat -v color.txt colorgnu.txt 
^[[0m^[[01;34ma^[[0m
^[[0m^[[01;34mb^[[0m
^[[0m^[[01;34ma^[[0m
^[[01;34mb^[[0m

@sylvestre
Copy link
Copy Markdown
Contributor Author

sylvestre commented Dec 6, 2023

The difference is that GNU ls does not immediately reset the text style after each directory name. It only resets (^[[0m) before starting a new colored text, or at the end of the output.

tertsdiepraam
tertsdiepraam reviewed Dec 6, 2023
View reviewed changes
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 6, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/ls/root-rel-symlink-color is no longer failing!
GNU test failed: tests/tail/inotify-dir-recreate. tests/tail/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 9, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/ls/multihardlink is no longer failing!
Congrats! The gnu test tests/ls/root-rel-symlink-color is no longer failing!
GNU test failed: tests/tail/inotify-dir-recreate. tests/tail/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

tertsdiepraam
tertsdiepraam approved these changes Dec 9, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tertsdiepraam tertsdiepraam left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice! The spell-check is failing but once that's fixed LGTM!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 9, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/ls/multihardlink is no longer failing!
Congrats! The gnu test tests/ls/root-rel-symlink-color is no longer failing!

@sylvestre sylvestre merged commit 0308e01 into uutils:main Dec 9, 2023
@sylvestre sylvestre deleted the gnu-legacy branch December 9, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cakebaker cakebaker cakebaker left review comments

@tertsdiepraam tertsdiepraam tertsdiepraam approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sylvestre @cakebaker @tertsdiepraam