feat(holiday-generator): support state holidays

[curbengh]

What is it?

• Bugfix
• Feature
• Codebase improvement

Description of the changes in your PR

• Improve on feat(holiday): australia regional holidays #378
• Re-add regional holidays removed by Holidays update #173
• Each state's calendar file also includes national holidays.
• source

Acknowledgement

• I read the contribution guidelines.

[curbengh]

ideally the states should be hidden and only shown when expanding "Australia" to avoid cluttering the list.

Something like this,

Australia

Australian Capital Territory

New South Wales

...

[tswistak]

Modification in the generator looks ok. However, it would be nice if for ID generation we were able to differentiate between state-specific and countrywide holidays. This way, if someone would like to have holidays for two states, they won't have duplicate entries for countrywide holidays.

Also, I think that in the future, two things should be done:

• keep countrywide holidays separate from state-specific to minimize space usage (especially useful if someone would do USA holidays)
• UI in Calendar should work similarly as you've mentioned. For Australia, it won't be the problem to have these few extra entries, but if someone would do USA holidays, it will bloat the whole list.

[curbengh]

date-holidays doesn't have a way to query state-specific holidays, so need a custom function for that (stateXHolidays = Holidays('AU', 'X') - Holidays('AU', 'X').

This way, if someone would like to have holidays for two states, they won't have duplicate entries for countrywide holidays.

Multiple states may share some holidays, so duplicates can't be completely avoided that way.

The following is apple's approach. For Australia holidays, state holidays are suffixed relevant state(s), e.g. SUMMARY;LANGUAGE=en:Labour Day (ACT\, NSW\, SA). For US holidays, state holidays are included but no suffix of observing states (not even in the DESCRIPTION line) and also incomplete (e.g. Lincoln's Birthday is missing). For Malaysia holidays, state holidays have no suffix but there are also interesting entries like

DESCRIPTION:(All states except Johor\, Kedah\, Kelantan\, Perlis & Terengganu)
SUMMARY;LANGUAGE=en-MY:New Year’s Day (observed)

None of the above approaches is suitable for every countries, programmatically include all approaches is not ideal either.

I think a better approach would be to keep using holiday-generator while still accepting community contribution (that also opt-out a country from holiday-generator). If a (country) holiday is out of date, we can always opt back in that country into automation.

I'm closing this in favor of #416 .

[tswistak]

I think a better approach would be to keep using holiday-generator while still accepting community contribution (that also opt-out a country from holiday-generator). If a (country) holiday is out of date, we can always opt back in that country into automation.

Now you did it for Australia and Malaysia. There's already Germany in the queue. There will probably also be requests for the United States, Canada, and who knows what else. Going this way, we may just remove the holiday generator and go back to the old times, where Calendar was relying only on contributors, and holidays weren't updated for years.

date-holidays doesn't have a way to query state-specific holidays, so need a custom function for that (stateXHolidays = Holidays('AU', 'X') - Holidays('AU', 'X').

In my opinion, in software development, there's no such thing as "it can't be done". Even if an external library has some limitations, there are always ways to omit them. For example, a great way (if we don't want to write any custom logic) would be to raise a feature request in the date-holidays and maybe even a pull request.

To conclude, I think that this PR should be reopened as it proposed a proper solution. And, PRs #416 and #422 should be closed.

CC @naveensingh

[naveensingh]

I thought about this and still prefer the approach I mentioned in my other comment:

• Create just one ICS file per country as it is now.
• Store all holidays along with their types in a X-HOLIDAY-TYPE field.
• Add another radio button dialog so users can choose between "All holidays" and "Public holidays".
• Only accept community contributions for countries that aren't already supported by date-holidays. Supported countries with missing holidays should be reported directly to upstream (either by us or the issue/PR author). Of course, the contribution guidelines will be updated for this.

This should resolve issues like #379 and #413 without complicating things too much. Google Calendar has a similar option:

X-HOLIDAY-TYPE:

Possible field values could be just:

• public (includes public and bank holidays like now)
• other

or it could be all the types from https://www.npmjs.com/package/date-holidays#types-of-holidays but merging them into just two types will keep things easy for those who are looking to add holidays for an unsupported country.

Alternative ideas and opinions are welcome. I'll probably work on this tomorrow unless someone beats me to it.

[tswistak]

It will solve the problem of non-public holidays, but what with regional holidays? They still can be public, but only for a specific region. Probably it would be the best to support all regions for countries that have them (according to https://www.npmjs.com/package/date-holidays#supported-countries-states-regions), not only for already reported ones.

Just an idea - maybe in the future, instead of ICS files, you could just build holidays from YML files from date-holidays (https://github.com/commenthol/date-holidays/tree/master/data/countries)? You could add them as a GIT Submodule. It will require writing a parser, but will be more flexible.

[naveensingh]

it would be the best to support all regions for countries that have them

As in, add a separate ICS file and UI option for each region like suggested in this comment?

I was thinking All holidays option should include all holidays (including those only observed in specific regions) for a given country.

build holidays from YML files

One step at a time ;)

[tswistak]

As in, add a separate ICS file and UI option for each region like suggested in #407 (comment)?

Something like this. Or, show another dialog with regions because e.g. USA will bloat the UI with their 53 regions (states + cities with separate holidays)

[naveensingh]

Ok, thanks! I'll think more about it. BTW, here's how Google is handling it:

(Holidays in Canada)