parse-cron
parse-cron copied to clipboard
siebertm
Strict Matching
This PR adds strict matching to cron parsing.
I find the default matching behavior when both DOW and DOM are specified to be unexpected. It inhibits a number of use cases, like matching Friday the 13th, or the second tuesday in a month (when i have philly.rb meetings!). So, I added an optional flag to support "strict matching" where both DOM and DOW must match.
Added spec coverage for behavior, and updated Readme to reflect. Happy to discuss any changes or make additional ones to get this merged in.
Hey Russell,
First of all: thanks for looking into that issue!
I aggree with your thinking that one should be able to express friday the 13th, but the spec itself doesn't allow this (as far as I can see).
Your idea of switching the "new" behaviour (why did you call it strict? Can't find that anywhere) with a flag looks sound to me.
I'll add general feedback to the code directly in the code.
@siebertm thanks for reviewing this! much appreciated.
I called it "strict" because it's a strict match. But you're right, it's made up. this is outside of the cron spec, but behavior that I wish it could do, instead of what it does now which I doubt anyone has ever actually used, because its a weird match condition to rely on.
@siebertm i updated the PR to remove the inverted if blocks. As for the removal of the check, and using attr_accessor, I was unable to get that to work sanely. It really does need to only be strict when both are wildcard, any other condition, with it enabled, can result in weird behavior.
Sorry for the long turn around! Let me know if there's anything else I need to update.