Allow "L" for end of the month in the "day" field of cron

[Congelli501]

New feature

This PR adds support for :

• L in the day field, to select the last day of the month
• negative number in the day field, to select a day indexed from the end of the month

L is just an alias for -1

L is used for end of month on many cron implementation (source: https://stackoverflow.com/questions/6139189/cron-job-to-run-on-the-last-day-of-the-month).

There are no bullet-proof ways to declare "end of month" with the basic cron feature (none that supports leap years).

Internal changes

To allow for the L alias, I transformed the alt Array into a Map.

[coveralls]

coverage: 100.0%. remained the same
when pulling 969e416 on blgCloud:master
into ef61a14 on roccivic:master.

[roccivic]

@Congelli501 Thank you for your contribution. Allow me some time to properly review your changes.

Meanwhile, one potential issue that I noticed, is that units is exposed publicly and changing the type of the alt property is a breaking change to the public api.

[Congelli501]

Meanwhile, one potential issue that I noticed, is that units is exposed publicly and changing the type of the alt property is a breaking change to the public api.

Indeed. I see two possible solutions:

• Revert most of the alias changes (revert the alt type to an array) and just handle the L alias in the code instead of using the alt system.
• Rename the new alt as aliases, with the type Map<string, number>, and keep the previous alt values as array, even if it is not used by the lib internals anymore.

[Congelli501]

@roccivic , what do you think about my proposal for units ? What would be your preferred approach ?

Did you have the chance to review the other changes ?

[roccivic]

I had a look at the pull request and I agree that it would be nice to support the L character for the last day of month value.

The current version does allow negative numbers and their use would throw an error. I'm not sure if anyone relies on the module to have this behaviour, but it could be a breaking change for someone.

I think that a great starting point would be to add a new option enableLastDayOfMonth to the Options type.

Afterwards, instead of updating the units, the implementation could be hardcoded where necessary, just like the option to display hashes. The option could specifically implement support for L in the string and -1 in the numerical representations.

As far as supporting arbitrary negative numbers. Would you to consider putting it behind another option, like enableNegativeDaysOfMonth?

Something like this: 4adf18c.

Have you checked for any edge cases with ranges of negative numbers? I haven't, but I wonder if the implementation handles these cases correctly since the range separator and the minus sign are the same character -. Could we add some tests that check values like -20--10?

[Congelli501]

The options argument is currently implemented only for arrayToString, not for the stringToArray nor getSchedule functions.

I'm not very found in adding a flag to enable the last day parsing, as it limits discoverability.

I made the following changes:

• don't allow negative number in parsing (string to array), only L will be allowed and parsed to -1. That adresses bad parsing risks you pointed out (like -20--10).
• only -1 is allowed in array mode for days (no other negative number)
• there are no more changes on units.ts

This should keep user facing changes to a mimimum.

I don't see a usecase for negative numbers other than L / -1 (last day of mounth), and I haven't seen any cron implemntation supporting it, so it's not a big loss.

What do you think about the last changes ? Do you prefer to add an option parameter to stringToArray (to allow L) and getSchedule (to allow -1) ?

[roccivic]

I'm not very found in adding a flag to enable the last day parsing, as it limits discoverability.

I think having an option for this feature is unavoidable. We don't really know which underlying cron implementation users are integrating with and whether it supports the L character. The user therefore needs to be able to disable the last day of month if their underlying system does not support it. https://en.wikipedia.org/wiki/Cron#Cron_expression

As far as the discoverability goes, that's only a matter of whether the option should be enabled or disabled by default.

[Congelli501]

I added the new parse option. The last day of month parsing is on by default.

[roccivic]

@Congelli501 Thank you for the contribution. I will publish a new version in the near future.

[Congelli501]

@roccivic , sorry to bother you, do you know when the release will be published?

[roccivic]

@Congelli501 No bother at all. I just updated the readme file with the changes from this pull request and realized that while the enableLastDayOfMonth options is available for stringToArray, it is missing from the arrayToString method where there is no way to disable the feature. I think this needs to be resolved before the release.

[roccivic]

@Congelli501 I published v2.1.0. Let me know if this work for you. Thanks

[Congelli501]

The timing allowed me to test it live.
It's working as expected, perfect.

[roccivic]

@Congelli501 That's great, thanks for checking.