floraison / fugit

time tools (cron, parsing, durations, ...) for Ruby, rufus-scheduler, and flor
MIT License
353 stars 29 forks source link

Fugit.parse incorrectly parsing am/pm #81

Closed ski-nine closed 1 year ago

ski-nine commented 1 year ago

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 commented 1 year ago
require 'fugit'
Fugit.parse('every day at 5:00 pm').original
# => "0 17 * * *"
jmettraux commented 1 year ago

@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.

ski-nine commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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