Skip to content

Support for parsing Crons with seconds#9

Closed
lukemurray wants to merge 3 commits intoshawnchin:masterfrom
lukemurray:master
Closed

Support for parsing Crons with seconds#9
lukemurray wants to merge 3 commits intoshawnchin:masterfrom
lukemurray:master

Conversation

@lukemurray
Copy link

Hey Shawn,

This isn't a great change but we use Quartz and wanted to use jquery-cron which would error on reading it.

I haven't made changes for the UI selection of seconds but it now can handle a cron schedule with 6 parts (first one being second).

If we get the time I might add some UI around selecting seconds I just haven't looked at it yet.

Cheers,
Luke

Luke Murray added 2 commits February 11, 2013 14:09
…st lets it correctly parse a 6 part cron with the first part being seconds. I needed this to support Quartz scheduling. Although currently there is no UI to support selecting seconds but you can now turn it on and read or output it with 0 seconds or add custom crons
@craigwblake
Copy link

Luke, thanks for the work to add support for Quartz. I pulled your commits to my fork and they seem to work well except for one case I've found. When I try to set the cron chooser from a string it does not seem to work with the ? instead of * in the day of week field, e.g.:

cron_field.cron('value', '0 * * * ?')

Throws an exception for me. Have you seen this behavior?

@lukemurray
Copy link
Author

Craig, I have. Sadly it is due to the way it validates combinations it has an object of regexs that it uses to figure out the type of cron to render the correct selectors. I just created the same array with seconds.

I've found it can't 'render' the selectors for many valid cron expressions, like #, / etc.

So my change only added support for seconds in the expression and selecting them via a customValue. Becuase Quartz also used the ? in the Day of Month & Day of Week I made it output the ? for blank. I never made it support ? as input.

I updated my fork to allow ? in the input, but I haven't really tested it.

Hope that helps.

@shawnchin
Copy link
Owner

Hi Luke. Many thanks for posting the request, and apologies for taking an unjustifiably long time to respond.

I appreciate why you're making these changes but I'm of two minds whether to incorporate this into the library or not. On one hand it would be nice to be able to support usage within Quartz, but on the other hand it's not quite aligned with the original purpose of jquery-cron i.e. keeping the interface as simple as possible for those not acquainted with cron expressions.

As you'd have noticed, the plugin was not designed to consume and validate arbitrary expressions (not to mention extended versions as used in Quartz), but only those that were generated by the plugin itself.

Extending the input validation to accept more formats would not be sufficient if the plugin itself cannot express or reproduce those values.

The point being that to properly support Quartz, the UI will need to also be enriched to represent some of the extended capabilities while keeping the interface intuitive and simple.

What do you think?

@lukemurray
Copy link
Author

That is OK, we all get busy.

I understand your thoughts and decision on what the plugin does, and respect that. I agree that keeping it simple is good, and making a UI that supports everything is hard. We fall back to a text input if it can't figure it out nicely and we run into that sometimes.

Just made a pull request in-case others needed the simple fix for seconds.

Thanks

@shawnchin shawnchin closed this Oct 13, 2013
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.

3 participants

Comments