Unix epoch string support #458
Conversation
…dotliquid into unix-epoch-strings
Codecov Report
@@ Coverage Diff @@
## master #458 +/- ##
==========================================
+ Coverage 87.27% 87.39% +0.11%
==========================================
Files 67 67
Lines 2632 2657 +25
Branches 633 642 +9
==========================================
+ Hits 2297 2322 +25
Misses 212 212
Partials 123 123
Continue to review full report at Codecov.
|
Added ConvertTime filter.
Added test coverage
|
Personally I think this needs to be broken down to three 'extended' filters:
The test cases would be greatly limited and the functionality understandable. Happy to debate naming of these filters. Also, date itself can depend on unix_sec to process numerics. |
|
@robin-parker please advise if you intend to implement my suggestion. I can't approve as is. |
…unix-epoch-strings
* unix_ms - convert epoch to DateTimeOffset * time_zone - convert date-time to another timezone
src/DotLiquid/ExtendedFilters.cs
Outdated
| { | ||
| // Convert the input to a DateTimeOffset | ||
| if (input is DateTimeOffset dateTimeOffset || | ||
| (input is string stringInput && DateTimeOffset.TryParse(stringInput, out dateTimeOffset))) |
There was a problem hiding this comment.
I don't agree with parsing in here, created extra overhead/testing.
There was a problem hiding this comment.
@microalps, the date filter returns a string, so without string parsing time_zone cannot be chained after a date filter, which limits it's usability.
How about if I changed it to use TryParseExact with a fixed format of say ISO-8601 with explicit timezone specifier.
That would reduce the overhead and simplify testing to a narrower set of input formats.
There was a problem hiding this comment.
Good idea. I'd accept it if it was limited in scope as you describe.
There was a problem hiding this comment.
Changes made and docs drafted at https://github.com/robin-parker/dotliquid/wiki/time_zone
| @@ -74,42 +74,30 @@ public void TestTimeZone() | |||
| { | |||
| Context context = new Context(CultureInfo.CurrentCulture); | |||
|
|
|||
| // Test UTC specified DateTime | |||
There was a problem hiding this comment.
These tests between the type should result in a different value. On a machine NOT in UTC, converting a datetime that is EXPLICITLY local to UTC would change the hour, but a datetime that is EXPLICITLY UTC would not. This functionality is missing I guess.
There was a problem hiding this comment.
@microalps, I'm not clear what functionality is missing. Could you advise/write a test case for the scenario you're thinking of?
There was a problem hiding this comment.
Say you have a hash that includes a date - from DateTime.Now - and the local machine is not in UTC. Calling TimeZone with this parameter and 'UTC' as the destination should change the time. HOWEVER, the same with DateTime.UtcNow should not as it is already in UTC.
date: adds support for UNIX Epoch timestamps as strings - resolves UNIX Epoch timestamp in strings ignored by Date filter. #457.unix_ms: new extended filter, which converts a UNIX Epoch timestamp in milliseconds to a DateTimeOffset.time_zone: new extended filter, which converts a DateTimeOffset or ISO-8601 date-string to a DateTimeOffset in a target timezone.Documentation