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 -f dates.txt is failing #4657

Closed
sylvestre opened this issue Mar 27, 2023 · 5 comments · Fixed by #6148
Closed

date -f dates.txt is failing #4657

sylvestre opened this issue Mar 27, 2023 · 5 comments · Fixed by #6148
Labels

Comments

@sylvestre
Copy link
Contributor

sylvestre commented Mar 27, 2023

$ echo "2023-03-27 08:30:00" > dates.txt
$ echo "2023-04-01 12:00:00" >> dates.txt
$ echo "2023-04-15 18:30:00" >> dates.txt

$ ./target/debug/coreutils date -f dates.txt "+%Y-%m-%d %H:%M:%S"
date: invalid date '2023-03-27 08:30:00'
date: invalid date '2023-04-01 12:00:00'
date: invalid date '2023-04-15 18:30:00'

With GNU:

2023-03-27 08:30:00
2023-04-01 12:00:00
2023-04-15 18:30:00
@tertsdiepraam
Copy link
Member

tertsdiepraam commented Mar 29, 2023

I think that the date parsing in touch is more advanced than in date. If we share that implementation this could be fixed quite easily. (They share implementations in GNU too)

But maybe that would be done after #4600

@Benjscho
Copy link
Contributor

Benjscho commented May 28, 2023

Time parsing in date and touch for GNU is provided by parse-datetime - which is a yacc implementation of parsing. Seems as though the main parsing order is as described here. Should be possible to implement parsing in the same order and features either through if let Ok() parse success matching (as is currently done in touch and your PR @tertsdiepraam), or potentially a yacc syntax rust parser using grmtools.

I'm going to look into providing a lib function that provides this kind of loose parsing and returns a DateTime object, will see if I can make something that can drop in to date. It could then be tested against gnulib/tests/test-parse-datetime.c. I'm guessing it would be best to avoid any further dependencies, so I'll try the first approach of result based successes.

@tertsdiepraam
Copy link
Member

tertsdiepraam commented May 28, 2023

Sounds good!

As a general precaution, keep this line of the contribution guidelines in mind:

uutils is original code. It cannot contain code from existing GNU or Unix-like utilities, nor should it link to or reference GNU libraries.

We have to copy the functionality without copying the code. Not looking directly at the code but only the documentation and observed behaviour is the best way to ensure that you don't (accidentally) copy it.

@Benjscho
Copy link
Contributor

Thanks, that's a good reminder! I'll steer clear from the implementation.

@Benjscho
Copy link
Contributor

Opened a PR that fixes this: #4917

@cakebaker cakebaker linked a pull request May 30, 2023 that will close this issue
mvo5 added a commit to mvo5/coreutils that referenced this issue Mar 30, 2024
This commit is a trivial followup for:
uutils#4917
and
uutils/parse_datetime#12

The functionality to parse the datetime was moved into the parse_datetime
crate and the only (tiny) piece left is to call it from `date`.

It also adds the two tests from the original PR#4917.

Closes: uutils#4657

Thanks to Ben Schofield
mvo5 added a commit to mvo5/coreutils that referenced this issue Mar 30, 2024
This commit is a trivial followup for:
uutils#4917
and
uutils/parse_datetime#12

The functionality to parse the datetime was moved into the parse_datetime
crate and the only (tiny) piece left is to call it from `date`.

It also adds the test-case from the original issue. I did not include
the two tests from PR#4917 because they appear to work even without
this change. I am happy to include them of course if prefered.

Closes: uutils#4657

Thanks to Ben Schofield
cakebaker pushed a commit that referenced this issue Mar 30, 2024
* date: fix `date -f dates.txt is failing`

This commit is a trivial followup for:
#4917
and
uutils/parse_datetime#12

The functionality to parse the datetime was moved into the parse_datetime
crate and the only (tiny) piece left is to call it from `date`.

It also adds the test-case from the original issue. I did not include
the two tests from PR#4917 because they appear to work even without
this change. I am happy to include them of course if prefered.

Closes: #4657

Thanks to Ben Schofield

* tests: tweak changes to test_date.rs to be more idiomatic

Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>

---------

Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
3 participants