From d480f32673db070848ae285db68546cd6b525820 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 24 Oct 2026 11:50:24 +0200 Subject: [PATCH] diff: use the traditional timestamp in context headers in the POSIX locale Should make test colors pass. cmp now also treats an empty LC_ALL or LANG as unset. --- src/cmp.rs | 20 ++----------- src/context_diff.rs | 12 ++++++-- src/macros.rs | 8 ++++-- src/unified_diff.rs | 7 +++-- src/utils.rs | 38 +++++++++++++++++++------ tests/by-util/test_cmp.rs | 42 ++++++++++++++++++++++++++++ tests/by-util/test_diff.rs | 57 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 151 insertions(+), 33 deletions(-) diff --git a/src/cmp.rs b/src/cmp.rs index 3ff716f0..82df9250 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -3,8 +3,8 @@ // For the full copyright and license information, please view the LICENSE-* // files that was distributed with this source code. -use crate::utils::format_failure_to_read_input_file; -use std::env::{self, ArgsOs}; +use crate::utils::{format_failure_to_read_input_file, is_posix_locale}; +use std::env::ArgsOs; use std::ffi::OsString; use std::io::{BufRead, BufReader, BufWriter, Read, Write}; use std::iter::Peekable; @@ -664,20 +664,6 @@ fn report_eof( } } -fn is_posix_locale() -> bool { - let locale = if let Ok(locale) = env::var("LC_ALL") { - locale - } else if let Ok(locale) = env::var("LC_MESSAGES") { - locale - } else if let Ok(locale) = env::var("LANG") { - locale - } else { - "C".to_string() - }; - - locale == "C" || locale == "POSIX" -} - #[inline] fn report_difference( from_byte: u8, @@ -690,7 +676,7 @@ fn report_difference( return; } - let term = if is_posix_locale() && !params.print_bytes { + let term = if is_posix_locale("LC_MESSAGES") && !params.print_bytes { "char" } else { "byte" diff --git a/src/context_diff.rs b/src/context_diff.rs index a37e7765..94c95be5 100644 --- a/src/context_diff.rs +++ b/src/context_diff.rs @@ -8,7 +8,7 @@ use std::io::Write; use crate::params::Params; use crate::utils::do_write_line; -use crate::utils::get_modification_time; +use crate::utils::{get_modification_time, is_posix_locale, TimestampStyle}; #[derive(Debug, PartialEq)] pub enum DiffLine { @@ -315,8 +315,14 @@ fn make_diff( #[must_use] pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Vec { - let from_modified_time = get_modification_time(¶ms.from.to_string_lossy()); - let to_modified_time = get_modification_time(¶ms.to.to_string_lossy()); + // The context format keeps the traditional timestamp in the POSIX locale. + let style = if is_posix_locale("LC_TIME") { + TimestampStyle::Traditional + } else { + TimestampStyle::Iso + }; + let from_modified_time = get_modification_time(¶ms.from.to_string_lossy(), style); + let to_modified_time = get_modification_time(¶ms.to.to_string_lossy(), style); let mut output = format!( "*** {0}\t{1}\n--- {2}\t{3}\n", params.from.to_string_lossy(), diff --git a/src/macros.rs b/src/macros.rs index 90a4eaaf..3bcc3aaf 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -1,5 +1,5 @@ // asserts equality of the actual diff and expected diff -// considering datetime varitations +// considering datetime varitations (ISO and traditional formats) // // It replaces the modification time in the actual diff // with placeholder "TIMESTAMP" and then asserts the equality @@ -17,7 +17,11 @@ macro_rules! assert_diff_eq { use std::str; let diff = str::from_utf8(&$actual).unwrap(); - let re = Regex::new(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [+-]\d{4}").unwrap(); + let re = Regex::new(concat!( + r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [+-]\d{4}", + r"|[A-Z][a-z]{2} [A-Z][a-z]{2} [ \d]\d \d{2}:\d{2}:\d{2} \d{4}" + )) + .unwrap(); let actual = re.replacen(diff, 2, "TIMESTAMP"); assert_eq!(actual, $expected); diff --git a/src/unified_diff.rs b/src/unified_diff.rs index bd83afe3..329a88ba 100644 --- a/src/unified_diff.rs +++ b/src/unified_diff.rs @@ -8,7 +8,7 @@ use std::io::Write; use crate::params::Params; use crate::utils::do_write_line; -use crate::utils::get_modification_time; +use crate::utils::{get_modification_time, TimestampStyle}; #[derive(Debug, PartialEq)] pub enum DiffLine { @@ -242,8 +242,9 @@ fn make_diff( #[must_use] pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Vec { - let from_modified_time = get_modification_time(¶ms.from.to_string_lossy()); - let to_modified_time = get_modification_time(¶ms.to.to_string_lossy()); + let from_modified_time = + get_modification_time(¶ms.from.to_string_lossy(), TimestampStyle::Iso); + let to_modified_time = get_modification_time(¶ms.to.to_string_lossy(), TimestampStyle::Iso); let mut output = format!( "--- {0}\t{1}\n+++ {2}\t{3}\n", params.from.to_string_lossy(), diff --git a/src/utils.rs b/src/utils.rs index d9415afd..ba35cc21 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -52,9 +52,29 @@ pub fn do_write_line( } } +/// How a modification time is rendered in a diff header. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum TimestampStyle { + /// `2026-03-05 07:08:09.500000000 +0100` + Iso, + /// `Thu Mar 5 07:08:09 2026` + Traditional, +} + +/// Returns whether the locale selected for `category` (e.g. `LC_TIME`) is the +/// POSIX one, following the `LC_ALL` > category > `LANG` precedence, where an +/// empty variable counts as unset. +pub fn is_posix_locale(category: &str) -> bool { + let locale = ["LC_ALL", category, "LANG"] + .into_iter() + .find_map(|name| std::env::var(name).ok().filter(|value| !value.is_empty())) + .unwrap_or_else(|| "C".to_string()); + locale == "C" || locale == "POSIX" +} + /// Retrieves the modification time of the input file specified by file path /// If an error occurs, it returns the current system time -pub fn get_modification_time(file_path: &str) -> String { +pub fn get_modification_time(file_path: &str, style: TimestampStyle) -> String { use chrono::{DateTime, Local}; use std::fs; use std::time::SystemTime; @@ -64,11 +84,11 @@ pub fn get_modification_time(file_path: &str) -> String { .unwrap_or(SystemTime::now()); let modification_time: DateTime = modification_time.into(); - let modification_time: String = modification_time - .format("%Y-%m-%d %H:%M:%S%.9f %z") - .to_string(); - - modification_time + let format = match style { + TimestampStyle::Iso => "%Y-%m-%d %H:%M:%S%.9f %z", + TimestampStyle::Traditional => "%a %b %e %H:%M:%S %Y", + }; + modification_time.format(format).to_string() } pub fn format_failure_to_read_input_file( @@ -277,7 +297,7 @@ mod tests { // verify assert_eq!( current, - get_modification_time(&temp.path().to_string_lossy()) + get_modification_time(&temp.path().to_string_lossy(), TimestampStyle::Iso) ); } @@ -292,7 +312,9 @@ mod tests { // Because the file is invalid, it will return SystemTime::now() // which will be greater than previously saved time let current_time: DateTime = SystemTime::now().into(); - let m_time: DateTime = get_modification_time(invalid_file).parse().unwrap(); + let m_time: DateTime = get_modification_time(invalid_file, TimestampStyle::Iso) + .parse() + .unwrap(); assert!(m_time > current_time); } diff --git a/tests/by-util/test_cmp.rs b/tests/by-util/test_cmp.rs index 0053a461..8392ede7 100644 --- a/tests/by-util/test_cmp.rs +++ b/tests/by-util/test_cmp.rs @@ -583,3 +583,45 @@ fn cmp_fast_paths() -> Result<(), Box> { Ok(()) } + +#[test] +fn cmp_difference_term_follows_messages_locale() -> Result<(), Box> { + let tmp_dir = tempdir()?; + let a_path = tmp_dir.path().join("a"); + let b_path = tmp_dir.path().join("b"); + std::fs::write(&a_path, "abc\n")?; + std::fs::write(&b_path, "bcd\n")?; + + // LC_ALL overrides LC_MESSAGES, which overrides LANG; empty values are unset. + for (variables, term) in [ + (&[][..], "char"), + ( + &[("LC_ALL", "C"), ("LC_MESSAGES", "en_US.UTF-8")][..], + "char", + ), + (&[("LC_ALL", "en_US.UTF-8"), ("LC_MESSAGES", "C")], "byte"), + (&[("LC_MESSAGES", "POSIX"), ("LANG", "en_US.UTF-8")], "char"), + (&[("LC_MESSAGES", "en_US.UTF-8"), ("LANG", "C")], "byte"), + (&[("LC_ALL", ""), ("LANG", "en_US.UTF-8")], "byte"), + ( + &[("LC_ALL", ""), ("LC_MESSAGES", ""), ("LANG", "C")], + "char", + ), + ] { + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.env_remove("LC_ALL") + .env_remove("LC_MESSAGES") + .env_remove("LANG") + .envs(variables.iter().copied()) + .arg("cmp") + .arg(&a_path) + .arg(&b_path); + cmd.assert() + .code(predicate::eq(1)) + .failure() + .stdout(predicate::str::ends_with(format!( + " differ: {term} 1, line 1\n" + ))); + } + Ok(()) +} diff --git a/tests/by-util/test_diff.rs b/tests/by-util/test_diff.rs index d745bde7..069fa15b 100644 --- a/tests/by-util/test_diff.rs +++ b/tests/by-util/test_diff.rs @@ -418,6 +418,63 @@ fn write_error() -> Result<(), Box> { Ok(()) } +// TZ is not honored on Windows. +#[cfg(unix)] +#[test] +fn context_timestamp_follows_time_locale() -> Result<(), Box> { + let tmp_dir = tempdir()?; + let old = tmp_dir.path().join("old"); + let new = tmp_dir.path().join("new"); + std::fs::write(&old, "left\n")?; + std::fs::write(&new, "right\n")?; + // 2009-02-13 23:31:30.25 UTC + let mtime = std::time::UNIX_EPOCH + std::time::Duration::from_millis(1_234_567_890_250); + for path in [&old, &new] { + File::options() + .write(true) + .open(path)? + .set_modified(mtime)?; + } + + let traditional = "Fri Feb 13 23:31:30 2009"; + let iso = "2009-02-13 23:31:30.250000000 +0000"; + // LC_ALL overrides LC_TIME, which overrides LANG; empty values are unset. + for (variables, timestamp) in [ + (&[("LC_ALL", "C")][..], traditional), + ( + &[("LC_ALL", "POSIX"), ("LC_TIME", "en_US.UTF-8")], + traditional, + ), + (&[("LC_ALL", "en_US.UTF-8"), ("LC_TIME", "C")], iso), + (&[("LC_TIME", "C"), ("LANG", "en_US.UTF-8")], traditional), + (&[("LC_TIME", "en_US.UTF-8"), ("LANG", "C")], iso), + ( + &[("LC_ALL", ""), ("LC_TIME", ""), ("LANG", "POSIX")], + traditional, + ), + (&[("LANG", "fr_FR.UTF-8")], iso), + ] { + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.env("TZ", "UTC0") + .env_remove("LC_ALL") + .env_remove("LC_TIME") + .env_remove("LANG") + .envs(variables.iter().copied()) + .arg("diff") + .arg("-c") + .arg(&old) + .arg(&new); + cmd.assert() + .code(predicate::eq(1)) + .stdout(predicate::str::starts_with(format!( + "*** {}\t{timestamp}\n--- {}\t{timestamp}\n", + old.display(), + new.display() + ))); + } + Ok(()) +} + #[test] fn compare_file_to_directory() -> Result<(), Box> { let tmp_dir = tempdir()?;