Re-work Value to "final" API, cleaning up problems along the way
#158
Conversation
This file contains 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
Template impact: There is now a consistent approach taken to when a `Value` can be used as a string, whole number, fractional number, bool etc. Examples - Every `Value` is now treated as a string. - Whole numbers are now preserved. - Whole numbers can convert to fractional numbers but not the other way around. - You can convert a fractional number to a whole number using the filters `round`, `ceil`, or `floor`. - Operations on only whole numbers preserve their whole number nature. - for tag ranges, indexing, etc only accept whole numbers Fixes cobalt-org#99 Programmatic impact: This implements the `Value` refactor from cobalt-org#95. `Value` is still an enum but now there is a single `Scalar` variant. This scalar is not an enum but is convertible between each of the common data types. Optimizations are in place to natively store some of these data types. BREAKING CHANGE: `Value`'s API has changed. Conversion behavior within templates has changed.
This was missed when adding support for `nil`. `compact` removes `nil`s from an array. This does not implement the optional parameter to filter out objects with a `nil` property.
This change attempts to strike a balance between what parts of the liquid API need to be dynamic and static. This makes the assumption that all block, tag, and filter names are known statically and uses a `&'static str` to represent them. Old ``` running 6 tests test bench_parse_template ... bench: 24,715 ns/iter (+/- 1,966) test bench_parse_text ... bench: 2,610 ns/iter (+/- 248) test bench_parse_variable ... bench: 4,628 ns/iter (+/- 425) test bench_render_template ... bench: 9,049 ns/iter (+/- 598) test bench_render_text ... bench: 2,204 ns/iter (+/- 151) test bench_render_variable ... bench: 6,867 ns/iter (+/- 437) ``` New ``` running 6 tests test bench_parse_template ... bench: 22,625 ns/iter (+/- 1,986) test bench_parse_text ... bench: 826 ns/iter (+/- 58) test bench_parse_variable ... bench: 2,871 ns/iter (+/- 175) test bench_render_template ... bench: 6,938 ns/iter (+/- 614) test bench_render_text ... bench: 432 ns/iter (+/- 29) test bench_render_variable ... bench: 6,201 ns/iter (+/- 537) ``` In the long term, we want to stop cloning `LiquidOptions` on every parse and `globals` on every render. This change reduced the impact of the former. BREAKING CHANGE: The API now takes `&'static str` instead of `String` for block, tag, and filter names
`date` but not `date_in_tz` was updated to parse more than one date format when cobalt switched formats. This updates `date_in_tz` to do so as well. This was done by centralizing the date parsing logic as discussed in cobalt-org#48.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This ended up causing significant performance improvement. It could have been more but for some reason the initial commit slowed down variables for some reason.