Skip to content

Conversation

@yuankunzhang
Copy link
Contributor

This is surprisingly more complex than I expected.

@yuankunzhang yuankunzhang force-pushed the tz-rule branch 2 times, most recently from 7ddd558 to 3139cfd Compare September 24, 2025 15:31
@yuankunzhang yuankunzhang marked this pull request as ready for review September 24, 2025 16:01
@cakebaker cakebaker merged commit e43652c into uutils:main Sep 25, 2025
17 checks passed
@codecov
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (5453d67) to head (f0c4995).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #232   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cakebaker
Copy link
Collaborator

Good work, thanks!

@Davoodeh
Copy link

Davoodeh commented Oct 20, 2025

Hello, thanks to everybody for the great work.

I'm sorry if this is not the right place to ask this.

I was studying this PR for a while and I was wondering that is there any reason (beyond performance) on why TZ="VALUE" is not relaxed with inner optional whitespaces (i.e TZ=" VALUE ")? Since such change passes all the current tests, this is a bit confusing to me.

This changes some behavior in an example like the following:

// prefixed whitespace
test(r#"TZ=" UTC-5:20:15""#, fixed_offset(0)); // current
test(r#"TZ=" UTC-5:20:15""#, fixed_offset(19215)); // with relaxed conditions

// prefixed whitespace
test(r#"TZ="UTC-5:20:15 ""#, Err(Backtrack(ContextError { context: [], cause: None }))); // current
test(r#"TZ="UTC-5:20:15 ""#, fixed_offset(19215)); // with relaxed conditions

Naturally, this can be generalized to the cases with :.

Whitespace relaxation can further improve in an unrelated example like: parse_datetime(" TZ=\"...\""). However in parse_datetime example, the user can easily just trim the input while removing the inner spaces of the quotation of this case is not as simple so that's why I think it's a quality improvement (if it is justified to begin with).

Example of changes that will relax this limitation:

diff --git a/src/items/timezone.rs b/src/items/timezone.rs
index 0414ee8..0009d17 100644
--- a/src/items/timezone.rs
+++ b/src/items/timezone.rs
@@ -16,6 +16,7 @@

 use jiff::tz::{Offset, TimeZone};
 use winnow::{
+    ascii::space0,
     combinator::{alt, delimited, opt, preceded, repeat},
     stream::AsChar,
     token::{one_of, take_while},
@@ -25,7 +26,12 @@ use winnow::{
 use super::primitive::{dec_uint, plus_or_minus};

 pub(super) fn parse(input: &mut &str) -> ModalResult<TimeZone> {
-    delimited("TZ=\"", preceded(opt(':'), alt((posix, iana))), '"').parse_next(input)
+    delimited(
+        ("TZ=\"", space0),
+        preceded(opt((':', space0)), alt((posix, iana))),
+        (space0, "\""),
+    )
+    .parse_next(input)
 }

 /// Parse a posix (proleptic) timezone string (e.g., "UTC7", "JST-9").

@sylvestre
Copy link
Contributor

@Davoodeh please open an issue to follow up here :)

@cakebaker
Copy link
Collaborator

@Davoodeh the reason it's not relaxed is that we follow the behavior of GNU date:

$ date -d 'TZ="America/New_York" 2025-10-20 14:00:00'
Mon Oct 20 08:00:00 PM CEST 2025
$ date -d 'TZ=" America/New_York" 2025-10-20 14:00:00'
Mon Oct 20 04:00:00 PM CEST 2025
$ date -d 'TZ="America/New_York " 2025-10-20 14:00:00'
Mon Oct 20 04:00:00 PM CEST 2025

@sylvestre
Copy link
Contributor

maybe it is something that GNU could improve :)

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.

4 participants