origin: record scalar-valued mapping keys as a sequence#15
Closed
reuvenharrison wants to merge 2 commits into
Closed
origin: record scalar-valued mapping keys as a sequence#15reuvenharrison wants to merge 2 commits into
reuvenharrison wants to merge 2 commits into
Conversation
addOrigin already records per-item locations for sequence-valued fields under a mapping's __origin__. Extend it to also record the keys of a scalar-valued mapping (a map of atomic values) into the same sequence slot, addressable by name. Such a mapping decodes into a Go map type that carries no Origin field of its own, so the per-key __origin__ injected on the nested mapping node is lost on decode. Recording the keys on the parent's __origin__, which does have somewhere to live, makes each entry's location reachable. Object-valued mappings are unaffected: each of their values is a mapping that already carries its own __origin__, so isScalarValuedMapping gates them out to avoid redundant data. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BqJA1X6suZYtR3tRbX8sLj
c5c3ef2 to
06913e3
Compare
This is a general-purpose YAML library, so its comments shouldn't lean on a particular consumer's domain. Drop the issue-tracker URLs and the endpoint/kin references from the origin comments, keeping the behavioral description each one carried. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BqJA1X6suZYtR3tRbX8sLj
Collaborator
Author
|
Superseded: no yaml3 change is needed. The per-map-entry key locations are already present in the OriginTree (verified against released v0.0.14); they were only being discarded downstream when a scalar-valued mapping decodes into a Go map with no Origin field. The fix belongs in kin-openapi's applyOrigins (the layer that knows a field is a map[string]scalar), which also avoids the over-firing this PR caused on schema objects. Closing in favor of the kin-side change. |
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.
What
addOriginalready records per-item locations for sequence-valued fields under a mapping's__origin__. This extends it to also record the keys of a scalar-valued mapping (a map of atomic values) into the same sequence slot, addressable by name.Why
A scalar-valued mapping, e.g.
decodes into a Go
map[string]Vwith scalarV, which carries noOriginfield of its own. The per-key__origin__injected on the nested mapping node is therefore lost on decode, so a consumer has no way to locate an individual entry. The only place a key's location can survive decode is on the parent object's origin, which does have somewhere to store it.Recording the keys as a sequence under the parent (reusing the existing
sequencesencoding, items addressable byname) makes each entry's location reachable by consumers that already readOrigin.Sequences.Object-valued mappings are unaffected: each of their values is itself a mapping that already carries its own
__origin__, soisScalarValuedMappinggates them out to avoid redundant data.Tests
TestOrigin_MapandTestOrigin_MapOfScalarsare updated to reflect the new sequence section (the parent now records the scalar-map keys);TestOrigin_MapOfScalarsgains a doc comment explaining the behavior. Full suite passes.