Merged
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
6f75bd1 to
4b99f91
Compare
This comment was marked as outdated.
This comment was marked as outdated.
…alue` Also move `From<Value> for serde_json::Value` to `srd/types/from.rs`.
4b99f91 to
fd14f61
Compare
This comment was marked as outdated.
This comment was marked as outdated.
fd14f61 to
b1f37e7
Compare
This comment was marked as outdated.
This comment was marked as outdated.
b1f37e7 to
e0cd521
Compare
This comment was marked as outdated.
This comment was marked as outdated.
e0cd521 to
4baf48d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
4baf48d to
06d1f53
Compare
This comment was marked as outdated.
This comment was marked as outdated.
06d1f53 to
23d73d8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
23d73d8 to
fdc75d8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
cec208b to
0e1a8f9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
0e1a8f9 to
1bf077e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
We treat `::` as the delimiter between reference path and default value. The implementation currently first resolves all nested references in both the reference and the default value parts before even looking for `:` and `::`. We always parse the default value as YAML to support complex default values (directly or via reference) and to provide the most intuitive default value behavior (any valid YAML snippets are used as typed values, and malformed snippets raise errors). more default value
We didn't have an `IntoIterator for Mapping` implementation for our Mapping type yet. This can be useful to minimize allocations if we're replacing a Mapping with another mapping while modifying only a fraction the contents.
… to `Value::Literal` Convert parsed string default value into Value::Literal to avoid incorrect additional reference resolution. We know that we've already resolved the top-level nested references at the time we call `extract_path_and_default`. Note that we don't want to recursively replace `Value::String` with `Value::Literal` so that default values which are references that expand to complex values still get resolved correctly.
…t value The debug messages are only printed out if `verbose_warnings` is set to `true` when rendering the inventory.
Add `README-extensions.md` to document reclass-rs specific extensions and update `README.md` to link to the new file.
1bf077e to
73ef9e7
Compare
Benchmark for 7a2368aClick to view benchmark
|
Member
Author
|
If you have a bit of time when reviewing please think about potentially missing test cases. |
bastjan
approved these changes
Dec 10, 2025
Member
bastjan
left a comment
There was a problem hiding this comment.
🥳
Did not find any additionally required test cases from the top of my head.
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
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.
We treat
::as the delimiter between reference path and default value. Note that this is technically a breaking change for existing reclass-rs inventories.The implementation currently first resolves all nested references in both the reference and the default value parts before even looking for
:and::.We always parse the default value as YAML to support complex default values (directly or via reference) and to provide the most intuitive default value behavior (any valid YAML snippets are used as typed values, and malformed snippets raise errors).
To make usage of simple default values more intuitive, we convert parsed string default value into
Value::Literalto avoid incorrect additional reference resolution. We know that we've already resolved the top-level nested references at the time we parse a default value.Note that we don't want to recursively replace
Value::StringwithValue::Literalso that default values which are references that expand to complex values still get resolved correctly.Generally, we recommend that users use references when they want to provide complex default values, since complex default values which are provided inline require special care to not confuse the reference and default parsers.
Checklist
bug,enhancement,documentation,change,breaking,dependency,internalas they show up in the changelog