Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

date: format date to correct date format before parsing #6401

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ahmadabd
Copy link
Contributor

closes #6392

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@RenjiSann
Copy link
Contributor

After looking at the code, there is this snippet in the parse_datetime_at_date function that checks 1999-1-4 against the ISO 8601 pattern.

For some reasons (I may not have all the context about it), we add "0000" to the candidate and "%H%M" to the string, which makes us check 1999-1-40000 against %Y-%m-%d%H%M.

I could understand why parsing 40000 as %d%H%M could fail, so maybe the error comes from here.

Maybe a workaround would be to add respectively " 0000" and " %H%M" to the candidate and pattern, so the parsing does not eat a 0 as part of the day.

This would require a change to the parse_datetime crate.

@ahmadabd
Copy link
Contributor Author

You mean the bug is in parse datetime crate? @RenjiSann

@RenjiSann
Copy link
Contributor

Yes, it most probably is.

@RenjiSann
Copy link
Contributor

The following example isolates the bug being on the parsing of the last single digit. It has no issue parsing -1- as a month.

use parse_datetime::parse_datetime;

fn main() {

    let one = parse_datetime("2001-01-04");
    let two = parse_datetime("2001-1-04");
    let three = parse_datetime("2001-01-4");
    let four = parse_datetime("2001-1-4");

    println!("Result:\n{:?}\n{:?}\n{:?}\n{:?}", one, two, three, four);
}
❯ cargo run
   Compiling test_parse_date_time v0.1.0 (/tmp/test_parse_date_time)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.47s
     Running `target/debug/test_parse_date_time`
Result:
Ok(2001-01-04T00:00:00+02:00)
Ok(2001-01-04T00:00:00+02:00)
Err(InvalidInput)
Err(InvalidInput)

@RenjiSann
Copy link
Contributor

Here is a PR for the fix in parse_datetime uutils/parse_datetime#76

@RenjiSann
Copy link
Contributor

Waiting for uutils/parse_datetime#76 to be released

@RenjiSann
Copy link
Contributor

#6423 was merged, from what I am testing, this is fixed.

Maybe we can still add tests to make sure there are no regressions afterwards ?

@ahmadabd do you want to give it a shot ? You can take a look at the tests I added in parse_datetime

@@ -222,6 +222,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
// Iterate over all dates - whether it's a single date or a file.
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

please run rustfmt

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.

date: YYYY-MM-DD parsed incorrectly
3 participants