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

dircolors: Move the static long string into structures. #5611

Merged
merged 13 commits into from Dec 7, 2023

Conversation

sylvestre
Copy link
Sponsor Contributor

This to be used in ls
and to simplify the code

AFAIK, it should not fix any gnu issue

@sylvestre sylvestre marked this pull request as ready for review December 2, 2023 14:43
tests/fixtures/dircolors/internal.expected Outdated Show resolved Hide resolved
src/uucore/src/lib/features/colors.rs Outdated Show resolved Hide resolved
src/uucore/src/lib/features/colors.rs Outdated Show resolved Hide resolved
src/uucore/src/lib/features/colors.rs Outdated Show resolved Hide resolved
src/uucore/src/lib/features/colors.rs Outdated Show resolved Hide resolved
src/uu/dircolors/src/dircolors.rs Outdated Show resolved Hide resolved
OutputFmt::Shell => "LS_COLORS='".to_string(),
OutputFmt::CShell => "setenv LS_COLORS '".to_string(),
OutputFmt::Display => String::new(),
OutputFmt::Unknown => unreachable!(),
Copy link
Member

Choose a reason for hiding this comment

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

We should open an issue after this PR to make OutputFmt an Option<OutputFmt> in dircolors so that this case can be eliminated.

src/uu/dircolors/src/dircolors.rs Outdated Show resolved Hide resolved
src/uu/dircolors/src/dircolors.rs Outdated Show resolved Hide resolved
sylvestre and others added 3 commits December 6, 2023 21:14
Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>
Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>
Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

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

Final comments otherwise seems ready!

Comment on lines +163 to +172
#[test]
fn test_print_ls_colors() {
new_ucmd!()
.pipe_in("OWT 40;33\n")
.args(&["--print-ls-colors"])
.succeeds()
.stdout_is("\x1B[40;33mtw\t40;33\x1B[0m\n")
.no_stderr();
}
*/
Copy link
Member

Choose a reason for hiding this comment

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

What's going on here? I'm not able to reproduce what this is supposed to do with GNU I think.

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.

This is tested in tests/misc/dircolors.pl

Copy link
Member

Choose a reason for hiding this comment

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

Hmm alright, I still haven't quite figured out how to reproduce it, but I've found the test

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.

reproduce this test?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I've been trying some commands with GNU dircolors to see how this is supposed to work, but it doesn't work for me. It seems to ignore whatever I pipe into it.

Comment on lines +201 to +212
/*
// Check if data is being piped into the program
if std::io::stdin().is_terminal() {
// No data piped, use default behavior
println!("{}", generate_ls_colors(&out_format, ":"));
return Ok(());
} else {
// Data is piped, process the input from stdin
let fin = BufReader::new(std::io::stdin());
result = parse(fin.lines().map_while(Result::ok), &out_format, "-");
}
*/
Copy link
Member

Choose a reason for hiding this comment

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

Should this be part of this PR?

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.

maybe ? :)
I am planning to work on this for tests/misc/dircolors.pl next

@sylvestre sylvestre merged commit fe730f8 into uutils:main Dec 7, 2023
53 of 55 checks passed
@sylvestre sylvestre deleted the dirdb branch December 7, 2023 08:58
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.

None yet

2 participants