Merge branch 'filter-by-regex'

[PhilipDaniels]

Fixes #39

• Implemented filtering using a regex in an environment variable CHRONO_TZ_BUILD_TIMEZONES
• Opted to do it all in this crate rather than pushing code into parse_info because I felt it was quite specific logic
• Updated README
• Build will panic if the regex is invalid
  generated_files.zip

It took me a while to get the filtering to work in what I consider a reasonable way - not helped by the fact that I think of the US as having timezones which are apparently deprecated according to Wikipedia (I wish I had found that table earlier). In some cases, the liberal filtering means that you end up with more timezones than you might strictly expect, but in all cases there is a lot less code generated than previously.

I've attached a zip with examples of directory.rs and timezones.rs for various regexes.

[quodlibetor]

Thanks so much for this!

I would love to have a test that verifies that this new behavior works correctly, I think that the most fine-tuneable way to do that would be to move build.rs into a chrono-tz-build crate inside this repo.

An alternative would be to add a couple tests that assert that some of the things do or don't get filtered out in the tests or examples directory, and run them from .github/workflows/rust.yml. If you'd be willing to add any tests I'd definitely appreciate it!

[PhilipDaniels]

@quodlibetor

I've added a small test project called check-regex-filtering which expects the environment variable to be set in a particular way, and done that from the rust.yml. There's an almost infinite variety of what might get included or excluded, I've tried to do a 'reasonable' set of checks without worrying too much about being precisely correct because I don't want tests to start failing due to future changes in IANA.

Incidentally, if you want to bikeshed the name of the environment variable, now is the time!

[quodlibetor]

This looks fantastic, exactly what I hoped for.

Could you either either document the way that this checks the recursive timezone check, or change the filter to filter out something that would only be filtered out if the filter applied recursively? For example, changing the filter to Europe and then asserting that at least some European countries do not appear. (Assuming that that's a correct implication of the hierarchy, I'm not particularly familiar with all the DB rules.)

[PhilipDaniels]

I've added a few more tests and some comments explaining what I think should be included and excluded. This stuff does get tricky...if we take your example of 'Europe' for example you get some Asian timezones - because of Istanbul! It all depends what's in the links, and I guess that is subject to change so we should be careful of how restrictive we are.

[quodlibetor]

Does it work to make this optional, and put it behind a "filter-timezones" feature? I'm genuinely not sure if you can cfg(feature = "filter-timezones") in a build.rs file.

[PhilipDaniels]

I can't swear to it, but I don't think it's possible. I tried making a commit here and it is easy enough to refactor chrono-tz itself to use the feature and conditionally compile out all the relevant regex code, but then using it from the test example doesn't seem to work. I think features are not passed through as one would want. This might be relevant:rust-lang/cargo#5499

[quodlibetor]

Amazing, Thanks!

[quodlibetor]

I'm going to hold off on merging this for just a bit while I get some other infrastructure in place, but it's fine to merge as-is and if anyone wants to experiment with it because they're running into #39 that seems reasonable to me.

[PhilipDaniels]

Thanks for the professional responses @quodlibetor I have no more pushes to make so merge when you're ready.

[quodlibetor]

Moving this into #69 with my infrastructure changes, hopefully I'll merge that today.