Recognizers-Text icon indicating copy to clipboard operation
Recognizers-Text copied to clipboard

[.NET Timexlib] Implement missing features of TimexProperty.ToString()

Open shana opened this issue 2 years ago • 3 comments

Following up on PR #2894 , this implements the missing features of TimexProperty.ToString(), adding support for date ranges, date + duration, etc.

This also fixes a bunch of off-by-one bugs when adding days or months to a Timex, where it would not take into account potential month/year overflows.

It also adds caching of the Types hashset of the Timex, so that it doesn't recalculate it every time the property is accessed. Instead, Types get cleared whenever a meaningful piece of data in the TimexProperty is changed.

Some tests have been converted to using DataRow instead of inline asserts, so that the test log will show the data being tested. It should also make it easier to see what the tests are doing. I also moved ToString tests to a separate class from the Roundtrip tests so it's easier to follow.

There's a lot of static helper methods that have been made into extension methods, because they're already in static classes so there's no overhead in doing it, and it makes caller code shorter and hopefully simpler to read. This has the side effect of making these DateTime extensions methods visible to consumers that include the TimexExpression namespace in the code - I don't know if that's undesirable or not, let me know if it's not something you want.

Fixes #2904

shana avatar Mar 15 '22 20:03 shana

Fyi, I've been away for Easter and life and couldn't follow up on this before, I should have more time now to get this thing done, it's not forgotten.

shana avatar Apr 19 '22 16:04 shana

Fyi, I've been away for Easter and life and couldn't follow up on this before, I should have more time now to get this thing done, it's not forgotten.

Thanks for the update, @shana!

tellarin avatar Apr 20 '22 06:04 tellarin

@shana Will you still update this PR? Looking forward to merging it.

tellarin avatar Jul 14 '22 03:07 tellarin