From 1db2e2356a6745a06f53c3ed4d3867900ae9a6c6 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 7 Jan 2025 22:33:57 +0100 Subject: [PATCH 1/3] dircolors: fix empty COLORTERM matching with ?* pattern should fix tests/misc/dircolors --- src/uu/dircolors/src/dircolors.rs | 15 ++++++++++++++- tests/by-util/test_dircolors.rs | 11 +++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/uu/dircolors/src/dircolors.rs b/src/uu/dircolors/src/dircolors.rs index faef0683e..9b57050f9 100644 --- a/src/uu/dircolors/src/dircolors.rs +++ b/src/uu/dircolors/src/dircolors.rs @@ -374,6 +374,7 @@ where let term = env::var("TERM").unwrap_or_else(|_| "none".to_owned()); let term = term.as_str(); + let colorterm = env::var("COLORTERM").unwrap_or_default(); let mut state = ParseState::Global; @@ -396,8 +397,20 @@ where )); } let lower = key.to_lowercase(); + if lower == "term" || lower == "colorterm" { - if term.fnmatch(val) { + let should_match = if lower == "colorterm" { + // For COLORTERM ?*, only match if COLORTERM is non-empty + if val == "?*" { + !colorterm.is_empty() + } else { + colorterm.fnmatch(val) + } + } else { + term.fnmatch(val) + }; + + if should_match { state = ParseState::Matched; } else if state != ParseState::Matched { state = ParseState::Pass; diff --git a/tests/by-util/test_dircolors.rs b/tests/by-util/test_dircolors.rs index 06d490c4a..909d904c4 100644 --- a/tests/by-util/test_dircolors.rs +++ b/tests/by-util/test_dircolors.rs @@ -253,3 +253,14 @@ fn test_repeated() { new_ucmd!().arg(arg).arg(arg).succeeds().no_stderr(); } } + +#[test] +fn test_colorterm_empty_with_wildcard() { + new_ucmd!() + .env("COLORTERM", "") + .pipe_in("COLORTERM ?*\nowt 40;33\n") + .args(&["-b", "-"]) + .succeeds() + .stdout_is("LS_COLORS='';\nexport LS_COLORS\n") + .no_stderr(); +} From 591cdc1d31447fa1fb9983410e6c20a2d74524e6 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 7 Jan 2025 22:49:28 +0100 Subject: [PATCH 2/3] dircolors: split the code to remove the clippy warning --- src/uu/dircolors/src/dircolors.rs | 122 ++++++++++++++++-------------- 1 file changed, 67 insertions(+), 55 deletions(-) diff --git a/src/uu/dircolors/src/dircolors.rs b/src/uu/dircolors/src/dircolors.rs index 9b57050f9..06e109e2b 100644 --- a/src/uu/dircolors/src/dircolors.rs +++ b/src/uu/dircolors/src/dircolors.rs @@ -8,7 +8,6 @@ use std::borrow::Borrow; use std::env; use std::fs::File; -//use std::io::IsTerminal; use std::io::{BufRead, BufReader}; use std::path::Path; @@ -360,8 +359,6 @@ enum ParseState { } use uucore::{format_usage, parse_glob}; - -#[allow(clippy::cognitive_complexity)] fn parse(user_input: T, fmt: &OutputFmt, fp: &str) -> Result where T: IntoIterator, @@ -372,11 +369,12 @@ where result.push_str(&prefix); + // Get environment variables once at the start let term = env::var("TERM").unwrap_or_else(|_| "none".to_owned()); - let term = term.as_str(); let colorterm = env::var("COLORTERM").unwrap_or_default(); let mut state = ParseState::Global; + let mut saw_colorterm_match = false; for (num, line) in user_input.into_iter().enumerate() { let num = num + 1; @@ -396,64 +394,38 @@ where num )); } - let lower = key.to_lowercase(); - if lower == "term" || lower == "colorterm" { - let should_match = if lower == "colorterm" { + let lower = key.to_lowercase(); + match lower.as_str() { + "term" => { + if term.fnmatch(val) { + state = ParseState::Matched; + } else if state == ParseState::Global { + state = ParseState::Pass; + } + } + "colorterm" => { // For COLORTERM ?*, only match if COLORTERM is non-empty - if val == "?*" { + let matches = if val == "?*" { !colorterm.is_empty() } else { colorterm.fnmatch(val) + }; + if matches { + state = ParseState::Matched; + saw_colorterm_match = true; + } else if !saw_colorterm_match && state == ParseState::Global { + state = ParseState::Pass; } - } else { - term.fnmatch(val) - }; - - if should_match { - state = ParseState::Matched; - } else if state != ParseState::Matched { - state = ParseState::Pass; } - } else { - if state == ParseState::Matched { - // prevent subsequent mismatched TERM from - // cancelling the input - state = ParseState::Continue; - } - if state != ParseState::Pass { - let search_key = lower.as_str(); - - if key.starts_with('.') { - if *fmt == OutputFmt::Display { - result.push_str(format!("\x1b[{val}m*{key}\t{val}\x1b[0m\n").as_str()); - } else { - result.push_str(format!("*{key}={val}:").as_str()); - } - } else if key.starts_with('*') { - if *fmt == OutputFmt::Display { - result.push_str(format!("\x1b[{val}m{key}\t{val}\x1b[0m\n").as_str()); - } else { - result.push_str(format!("{key}={val}:").as_str()); - } - } else if lower == "options" || lower == "color" || lower == "eightbit" { - // Slackware only. Ignore - } else if let Some((_, s)) = FILE_ATTRIBUTE_CODES - .iter() - .find(|&&(key, _)| key == search_key) - { - if *fmt == OutputFmt::Display { - result.push_str(format!("\x1b[{val}m{s}\t{val}\x1b[0m\n").as_str()); - } else { - result.push_str(format!("{s}={val}:").as_str()); - } - } else { - return Err(format!( - "{}:{}: unrecognized keyword {}", - fp.maybe_quote(), - num, - key - )); + _ => { + if state == ParseState::Matched { + // prevent subsequent mismatched TERM from + // cancelling the input + state = ParseState::Continue; + } + if state != ParseState::Pass { + append_entry(&mut result, fmt, key, &lower, val)?; } } } @@ -468,6 +440,46 @@ where Ok(result) } +fn append_entry( + result: &mut String, + fmt: &OutputFmt, + key: &str, + lower: &str, + val: &str, +) -> Result<(), String> { + if key.starts_with(['.', '*']) { + let entry = if key.starts_with('.') { + format!("*{key}") + } else { + key.to_string() + }; + let disp = if *fmt == OutputFmt::Display { + format!("\x1b[{val}m{entry}\t{val}\x1b[0m\n") + } else { + format!("{entry}={val}:") + }; + result.push_str(&disp); + return Ok(()); + } + + match lower { + "options" | "color" | "eightbit" => Ok(()), // Slackware only, ignore + _ => { + if let Some((_, s)) = FILE_ATTRIBUTE_CODES.iter().find(|&&(key, _)| key == lower) { + let disp = if *fmt == OutputFmt::Display { + format!("\x1b[{val}m{s}\t{val}\x1b[0m\n") + } else { + format!("{s}={val}:") + }; + result.push_str(&disp); + Ok(()) + } else { + Err(format!("unrecognized keyword {key}")) + } + } + } +} + /// Escape single quotes because they are not allowed between single quotes in shell code, and code /// enclosed by single quotes is what is returned by `parse()`. /// From 106873171d4f4a195d09da653ec637f7c16cbb99 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 14 Jan 2025 23:24:38 +0100 Subject: [PATCH 3/3] dircolors: ignore spell issues --- src/uu/dircolors/src/dircolors.rs | 2 +- tests/by-util/test_dircolors.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/dircolors/src/dircolors.rs b/src/uu/dircolors/src/dircolors.rs index 06e109e2b..c5b2e49f9 100644 --- a/src/uu/dircolors/src/dircolors.rs +++ b/src/uu/dircolors/src/dircolors.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) clrtoeol dircolors eightbit endcode fnmatch leftcode multihardlink rightcode setenv sgid suid colorterm +// spell-checker:ignore (ToDO) clrtoeol dircolors eightbit endcode fnmatch leftcode multihardlink rightcode setenv sgid suid colorterm disp use std::borrow::Borrow; use std::env; diff --git a/tests/by-util/test_dircolors.rs b/tests/by-util/test_dircolors.rs index 909d904c4..ffabe2923 100644 --- a/tests/by-util/test_dircolors.rs +++ b/tests/by-util/test_dircolors.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 overridable +// spell-checker:ignore overridable colorterm use crate::common::util::TestScenario; use dircolors::{guess_syntax, OutputFmt, StrUtils};