Incorrect handling of comma-separated values with ranges and/or stepping

[egfx-notifications]

Hi, I'm currently evaluating cron-related libraries for usage in my project, including reviewing the code of any candidates before deciding which one to use.
Let me first say that I'm very pleased with the quality of your code and that I'm very likely to choose this project to handle cron schedules in my application.

During code review I stumbled upon a few lines regarding the handling of a pattern which as far as I know reject patterns that would be perfectly valid in cron syntax and would also successfully parse by just removing those checks. These lines were introduced with 62c0289

croner/src/pattern.js

Lines 141 to 163 in 12cc992

	const split = conf.split(",");
	if( split.length > 1 ) {
	for( let i = 0; i < split.length; i++ ) {
	this.partToArray(type, split[i], valueIndexOffset, true);
	}
	// Handle range with stepping (x-y/z)
	} else if( conf.indexOf("-") !== -1 && conf.indexOf("/") !== -1 ) {
	if (recursed) throw new Error("CronPattern: Range with stepping cannot coexist with ,");
	this.handleRangeWithStepping(conf, type, valueIndexOffset);
	// Handle range
	} else if( conf.indexOf("-") !== -1 ) {
	if (recursed) throw new Error("CronPattern: Range with stepping cannot coexist with ,");
	this.handleRange(conf, type, valueIndexOffset);
	// Handle stepping
	} else if( conf.indexOf("/") !== -1 ) {
	if (recursed) throw new Error("CronPattern: Range with stepping cannot coexist with ,");
	this.handleStepping(conf, type, valueIndexOffset);

As you can see in the snippet above, in the function CronPattern.partToArray() you first split parts of the current pattern part by comma, but then reject any ranges or steppings if they are evaluated after this split, in the recursion. Contrary to the statement of the thrown error, comma-separated values can perfectly coexist with ranges and stepping.

I entered the examples from your tests for Cronitor and it parses well:
https://crontab.guru/#1,2/5__**
https://crontab.guru/#1-15/5,6__**
https://crontab.guru/#1-15,17__**

As you can see from the website's explanation these patterns are perfectly fine and if you'd just remove those checks they would indeed parse as intended. Actually you could get rid of the recursed parameter altogether.

Is there any reason I didn't see yet why you reject such patterns? If you want to accept these patterns as Cronitor does, I can submit a PR as I already made some tests locally. Just want to hear your opinion on it first.

[Hexagon]

Sounds like a great fix, and something i just haven't thought of.

Submit the PR when you're happy with it, and I'll have a deeper look asap 👍

[Hexagon]

Great finding, and great work. Thanks!

Released in 4.3.8

[Hexagon]

@egfx-notifications

One additional note, since you mention crontab.guru.

It seems like this site use standard mode when combining day-of-month with day-of-week. Croner works a bit different by default, which I've explained at #53. I am considering to change default behavior on next major release, but for now you need to use option legacyMode: true.

[egfx-notifications]

Yes, I noticed that during review, but thanks for pointing it out :)