From f6d3f9afbfb47ad9752c0cfe6ee21c459b56c615 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 15 Dec 2023 22:49:35 +0100 Subject: [PATCH 1/4] ls: manages the COLOR and COLORTERM variables Should fix GNU tests/ls/color-term.sh --- src/uu/ls/src/ls.rs | 33 +++++++++++++++++++++- tests/by-util/test_ls.rs | 61 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 1fceefe17..424ef8cc1 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype +// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm use clap::{ builder::{NonEmptyStringValueParser, ValueParser}, @@ -553,12 +553,43 @@ fn extract_time(options: &clap::ArgMatches) -> Time { } } +// Some env variables can be passed +// For now, we are only verifying if empty or not and known for TERM +fn is_color_compatible_term() -> bool { + let is_term_set = std::env::var("TERM").is_ok(); + let is_colorterm_set = std::env::var("COLORTERM").is_ok(); + + let term = std::env::var("TERM").unwrap_or_default(); + let colorterm = std::env::var("COLORTERM").unwrap_or_default(); + + // Search function to manage the "*" into the data structure + let term_matches = |term: &str| -> bool { + uucore::colors::TERMS.iter().any(|&pattern| { + term == pattern + || (pattern.ends_with('*') && term.starts_with(&pattern[..pattern.len() - 1])) + }) + }; + + if is_term_set && colorterm.is_empty() && is_colorterm_set && term.is_empty() { + return false; + } + + if !term.is_empty() && !term_matches(&term) { + return false; + } + true +} + /// Extracts the color option to use based on the options provided. /// /// # Returns /// /// A boolean representing whether or not to use color. fn extract_color(options: &clap::ArgMatches) -> bool { + if !is_color_compatible_term() { + return false; + } + match options.get_one::(options::COLOR) { None => options.contains_id(options::COLOR), Some(val) => match val.as_str() { diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index c3460633a..ca1da8b7e 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (words) READMECAREFULLY birthtime doesntexist oneline somebackup lrwx somefile somegroup somehiddenbackup somehiddenfile tabsize aaaaaaaa bbbb cccc dddddddd ncccc neee naaaaa nbcdef nfffff dired subdired tmpfs mdir +// spell-checker:ignore (words) READMECAREFULLY birthtime doesntexist oneline somebackup lrwx somefile somegroup somehiddenbackup somehiddenfile tabsize aaaaaaaa bbbb cccc dddddddd ncccc neee naaaaa nbcdef nfffff dired subdired tmpfs mdir COLORTERM mexe #[cfg(any(unix, feature = "feat_selinux"))] use crate::common::util::expected_result; @@ -3988,3 +3988,62 @@ fn test_ls_color_do_not_reset() { "\\u{1b}[0m\\u{1b}[01;34ma\\u{1b}[0m\\n\\u{1b}[01;34mb\\u{1b}[0m\\n" ); } + +#[cfg(all(unix, feature = "chmod"))] +#[test] +fn test_term_colorterm() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("exe"); + scene.ccmd("chmod").arg("+x").arg("exe").succeeds(); + + // Should show colors + let result = scene + .ucmd() + .arg("--color=always") + .env("LS_COLORS", "") + .env("TERM", "") + .succeeds(); + assert_eq!( + result.stdout_str().trim().escape_default().to_string(), + "\\u{1b}[0m\\u{1b}[01;32mexe\\u{1b}[0m" + ); + + // Should show colors + let result = scene + .ucmd() + .arg("--color=always") + .env("LS_COLORS", "") + .env("COLORTERM", "") + .succeeds(); + assert_eq!( + result.stdout_str().trim().escape_default().to_string(), + "\\u{1b}[0m\\u{1b}[01;32mexe\\u{1b}[0m" + ); + + // No colors + let result = scene + .ucmd() + .arg("--color=always") + .env("LS_COLORS", "") + .env("TERM", "") + .env("COLORTERM", "") + .succeeds(); + assert_eq!( + result.stdout_str().trim().escape_default().to_string(), + "exe" + ); + + // No colors + let result = scene + .ucmd() + .arg("--color=always") + .env("LS_COLORS", "") + .env("TERM", "dumb") + .env("COLORTERM", "") + .succeeds(); + assert_eq!( + result.stdout_str().trim().escape_default().to_string(), + "exe" + ); +} From 9167a843586d9a27402eb380ebe195df507c9161 Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Sat, 16 Dec 2023 17:17:44 +0100 Subject: [PATCH 2/4] ls: enable "colors" feature --- src/uu/ls/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index a82a1f37e..d11eeb27c 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -24,6 +24,7 @@ terminal_size = { workspace = true } glob = { workspace = true } lscolors = { workspace = true } uucore = { workspace = true, features = [ + "colors", "entries", "fs", "quoting-style", From af2625c8ce64cd4ddd406b1d86c4b056d8f64ebb Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 17 Dec 2023 17:39:02 +0100 Subject: [PATCH 3/4] fix order Co-authored-by: Daniel Hofstetter --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 424ef8cc1..77f791342 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -570,7 +570,7 @@ fn is_color_compatible_term() -> bool { }) }; - if is_term_set && colorterm.is_empty() && is_colorterm_set && term.is_empty() { + if is_term_set && term.is_empty() && is_colorterm_set && colorterm.is_empty() { return false; } From 5b451599961043de7e31c2e43f5bfe714416c218 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 17 Dec 2023 17:40:44 +0100 Subject: [PATCH 4/4] Improve the comment --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 77f791342..bfce1db79 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -562,7 +562,7 @@ fn is_color_compatible_term() -> bool { let term = std::env::var("TERM").unwrap_or_default(); let colorterm = std::env::var("COLORTERM").unwrap_or_default(); - // Search function to manage the "*" into the data structure + // Search function in the TERM struct to manage the wildcards let term_matches = |term: &str| -> bool { uucore::colors::TERMS.iter().any(|&pattern| { term == pattern