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

Fugit.parse incorrectly parsing am/pm #81

Closed
ghost opened this issue Nov 3, 2022 · 5 comments
Closed

Fugit.parse incorrectly parsing am/pm #81

ghost opened this issue Nov 3, 2022 · 5 comments
Assignees

Comments

@ghost
Copy link

ghost commented Nov 3, 2022

Issue description

When parsing a string with a time value where the "pm" is not space delimited, the result is being parsed as a time in the morning.

Also, wrong result when parsing a string with a time value of "12:xx am".

How to reproduce

PM Bug:

require 'fugit'
Fugit.parse('every day at 5:00pm').original
=> "0 5 * * *"

12 AM/PM Bug:

[55] pry(main)> Fugit.parse('every day at 12:15 am').original
=> "15 12 * * *"
[56] pry(main)> Fugit.parse('every day at 12:15 pm').original
=> "15 12 * * *"

Expected behaviour

PM Bug:

Either fix to return:

=> "0 17 * * *"

12 AM/PM Bug:

[55] pry(main)> Fugit.parse('every day at 12:15 am').original
=> "15 0 * * *"
[56] pry(main)> Fugit.parse('every day at 12:15 pm').original
=> "15 12 * * *"

OR

Raise a parse exception.

@jmettraux jmettraux self-assigned this Nov 3, 2022
@jmettraux
Copy link
Member

require 'fugit'
Fugit.parse('every day at 5:00 pm').original
# => "0 17 * * *"

@jmettraux
Copy link
Member

@ski-nine

Regarding 12am and 12pm, please read https://en.wikipedia.org/wiki/12-hour_clock#Confusion_at_noon_and_midnight

I will most likely keep the current behaviour, but add "12 noon" and "12 midnight" for clarification.

@ghost
Copy link
Author

ghost commented Nov 3, 2022

If not fixing then the behaviour should be documented in the main page because of the potential to cause problems.

We are using Rufus scheduler https://github.com/jmettraux/rufus-scheduler and some jobs were incorrectly scheduled due to the above issues.

@jmettraux
Copy link
Member

If not fixing then the behaviour should be documented in the main page because of the potential to cause problems.

From my point of view, it is not "not fixing". Yes, I will document my decision, thanks for the suggestion.

jmettraux added a commit that referenced this issue Nov 3, 2022
@jmettraux
Copy link
Member

Just released fugit 1.7.2. Thanks for reporting those two issues.

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

No branches or pull requests

1 participant