Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/uu/ls/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1130,13 +1130,13 @@ fn parse_time_style(options: &clap::ArgMatches) -> Result<(String, Option<String
// `field` can be empty here (e.g. --time-style=posix-), so test
// the prefix instead of unwrapping the first char.
_ if field.starts_with('+') => {
// recent/older formats are (optionally) separated by a newline
// The non-recent format comes first, followed by the recent format.
let mut it = field[1..].split('\n');
let recent = it.next().unwrap_or_default();
let older = it.next();
match it.next() {
None => ok((recent, older)),
Some(_) => Err(LsError::TimeStyleParseError(String::from(field))),
let older = it.next().unwrap_or_default();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

older is passed as the recent format just below, could you rename it to first? it reads wrong otherwise

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Renamed the first parsed format to first in c0212f7. It is used for both timestamps when there is no newline, and for older timestamps when there is one.

match (it.next(), it.next()) {
(None, None) => ok((older, None)),
(Some(recent), None) => ok((recent, Some(older))),
(_, Some(_)) => Err(LsError::TimeStyleParseError(String::from(field))),
}
}
_ => Err(LsError::TimeStyleParseError(String::from(field))),
Expand Down
51 changes: 45 additions & 6 deletions tests/by-util/test_ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2397,13 +2397,13 @@ fn test_ls_time_styles() {
.stdout_matches(&re_custom_format_recent)
.stdout_matches(&re_custom_format_old);

//+FORMAT_RECENT\nFORMAT_OLD
//+FORMAT_OLD\nFORMAT_RECENT
let re_custom_format_old =
Regex::new(r"[a-z-]* \d* [\w.]* [\w.]* \d* \d{4}--\d{2} test-old\n").unwrap();
scene
.ucmd()
.arg("-l")
.arg("--time-style=+%Y__%M\n%Y--%M")
.arg("--time-style=+%Y--%M\n%Y__%M")
.succeeds()
.stdout_matches(&re_custom_format_recent)
.stdout_matches(&re_custom_format_old);
Expand All @@ -2419,7 +2419,7 @@ fn test_ls_time_styles() {
scene
.ucmd()
.arg("-l")
.arg("--time-style=+%Y__%M\n%Y--%M\n")
.arg("--time-style=+%Y--%M\n%Y__%M\n")
.fails_with_code(2);

//Overwrite options tests
Expand Down Expand Up @@ -2534,12 +2534,12 @@ fn test_ls_time_recent_future() {
.stdout_matches(&re_iso_old);

// Also test that we can set a format that varies for recent of older files.
//+FORMAT_RECENT\nFORMAT_OLD
//+FORMAT_OLD\nFORMAT_RECENT
f.set_modified(SystemTime::now()).unwrap();
scene
.ucmd()
.arg("-l")
.arg("--time-style=+RECENT\nOLD")
.arg("--time-style=+OLD\nRECENT")
.succeeds()
.stdout_contains("RECENT");

Expand All @@ -2549,7 +2549,7 @@ fn test_ls_time_recent_future() {
scene
.ucmd()
.arg("-l")
.arg("--time-style=+RECENT\nOLD")
.arg("--time-style=+OLD\nRECENT")
.succeeds()
.stdout_contains("OLD");

Expand Down Expand Up @@ -8250,3 +8250,42 @@ ls: invalid --block-size argument '1fb'
.stderr_is("ls: invalid --block-size argument '1fb'\n");
}
}

#[test]
fn test_ls_custom_time_style_recent_and_older() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this overlaps quite a bit with test_ls_time_styles, could the new empty-half cases go there instead? thanks

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Moved the empty-half and environment cases into test_ls_time_styles in c0212f7, reusing its recent and old fixtures. The focused test and clippy pass.

let scene = TestScenario::new(util_name!());
scene.fixtures.touch("recent");
scene
.fixtures
.make_file("older")
.set_modified(std::time::UNIX_EPOCH)
.unwrap();

for (style, expected_recent, expected_older) in [
("+OLD\nNEW", "NEW", "OLD"),
("+\nNEW", "NEW", ""),
("+OLD\n", "", "OLD"),
("+SAME", "SAME", "SAME"),
] {
for use_env in [false, true] {
for (file, expected) in [("recent", expected_recent), ("older", expected_older)] {
let mut cmd = scene.ucmd();
cmd.arg("-ln");
if use_env {
cmd.env("TIME_STYLE", style);
} else {
cmd.arg(format!("--time-style={style}"));
}
let result = cmd.arg(file).succeeds();
let fields: Vec<_> = result.stdout_str().split_ascii_whitespace().collect();
assert_eq!(fields.last(), Some(&file), "unexpected output: {fields:?}");
if expected.is_empty() {
assert_eq!(fields.len(), 6, "unexpected output: {fields:?}");
} else {
assert_eq!(fields.len(), 7, "unexpected output: {fields:?}");
assert_eq!(fields[5], expected, "unexpected output: {fields:?}");
}
}
}
}
}
Loading