search

shawnchin / jquery-cron

A jQuery plugin to provide a simplified interface for users to specify cron entries
http://shawnchin.github.io/jquery-cron
MIT License
177 stars 118 forks source link

Support for parsing Crons with seconds #9

Closed lukemurray closed 10 years ago

lukemurray commented 11 years ago

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

craigwblake commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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