Fill in DateTimeFormatterBuilder, and apply a parsed offset - #5
Open
yogthos wants to merge 1 commit into
Open
Conversation
malli.transform builds its inst decoder with the builder API — appendFraction, appendOffset, parseDefaulting, optionalStart/End — and only appendPattern and appendLiteral were here, so the namespace could not load. The builder composes a pattern string, which the formatter and parse-with-pattern already speak, so each append* maps to the letters meaning the same thing: appendFraction to S's behind an optional decimal point, appendOffset to the generic X (every JVM offset pattern is the same run of sign/digits/colons to this parser), optionalStart/End to the pattern language's own [ … ]. parseDefaulting records the fallback and returns the builder; parse-with-pattern already starts each field at the JVM's default. Optional sections needed real support in the parser: [ … ] used to fall through to the plain-literal arm, which consumed an input character per bracket and threw the walk out of alignment. A section is now attempted and, if a literal does not match or a numeric field finds no digits, skipped whole. Sections nest, since appendFraction with a zero minimum brackets itself and callers wrap it again. That strictness applies only inside a section — at the top level literals stay lenient, so patterns that parsed before parse identically. Two fixes fell out of testing it against reference java.time: - .parse on an offset-bearing pattern now returns an OffsetDateTime, so the offset survives into (Instant/from (.parse f s)). It was returning a LocalDateTime, which read "…T13:45:30+0200" as 13:45 UTC instead of 11:45. - a fraction scales by the digits actually present, not the pattern's width, so a 9-wide field given ".417" is milliseconds rather than nanoseconds. ChronoField is deliberately not registered here: jolt core already models it as a real enum, and re-registering the statics shadowed it with something weaker. Gate: 6 assertions in fmt_test, values certified against JDK 20. 102 tests, 1043 assertions, no regression (1037 before, same 99 tests).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
malli.transform builds its inst decoder with the builder API —
appendFraction,appendOffset,parseDefaulting,optionalStart/optionalEnd— and onlyappendPatternandappendLiteralwere implemented, so the namespace could not load at all.The builder
It composes a pattern string, which the formatter and
parse-with-patternalready speak, so eachappend*maps to the letters that mean the same thing:appendFraction→Ss behind an optional decimal pointappendOffset→ the genericX(every JVM offset pattern —+HHMM,+HH:MM:ss,+HHMMss— is the same run of sign/digits/colons to this parser, and the no-offset text is theZit already accepts)optionalStart/optionalEnd→ the pattern language's own[ … ]parseDefaultingrecords the fallback and returns the builder;parse-with-patternalready starts each field at the JVM's defaultOptional sections
[ … ]needed real parser support. It used to fall through to the plain-literal arm, which consumed an input character per bracket and threw the whole walk out of alignment. A section is now attempted and, if a literal does not match or a numeric field finds no digits, skipped whole. Sections nest, sinceappendFractionwith a zero minimum brackets itself and callers commonly wrap it again.That strictness applies only inside a section — at the top level literals stay lenient, so every pattern that parsed before parses identically.
Two bugs found while certifying against reference java.time
.parsedropped a parsed offset. It returned aLocalDateTime, so(Instant/from (.parse f "2020-03-05T13:45:30+0200"))read 13:45 UTC instead of 11:45. An offset-bearing pattern now parses to anOffsetDateTime; a pattern without an offset letter still gives aLocalDateTime, which is what tick'sparse-*expect..417came out as nanoseconds instead of milliseconds.ChronoFieldis deliberately not registered here — jolt core already models it as a real enum (jolt.time.enums), and re-registering the statics shadowed that with something weaker. Caught because it broke 10 existing assertions.Gate
6 assertions in
fmt_test, values certified against JDK 20, including the full malli-shaped formatter parsing with every optional section present, with only the fraction, with only the offset, and with none.102 tests / 1043 assertions / 0 fail / 0 error. No regression — the 99 pre-existing tests and their 1037 assertions are unchanged, verified by running the suite with the source change stashed.