-
-
Notifications
You must be signed in to change notification settings - Fork 357
Clarify "$recursiveAnchor" behavior in terms of base URIs #795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
jsonschema-core.xml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming there should be a closing bracket here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, you don't like infinite asides?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
jsonschema-core.xml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"as a dynamic reference target (by use of $ref)"
Is this what you mean by that phrase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite, but I changed the wording so it should be more clear now.
One quick fix. One query.
Looking at the example makes the use clear, but I SHOULD be able to mostly understand it without the example. I'm not 100% confident that's the case. I'll re-read a few more times.
I can understand why $recursiveAnchor and $recursiveRef are in individual sections, but I think in doing so you've made it really hard to understand before reading the example. I can't quite match up the spec words to the way in the process works and is explained in the example.
The example is great BTW!
Maybe the spec could detail a step by step algorithm for resolving $recursiveRef?
This could be a new section, or as part of a merged those two keywords section (if you decide to try that).
I'll say it again, the example makes it really clear. I've not looked at this for a while, but looking at the example now, I immediatly understand how it works. Such quick understanding is really encouraging and makes me (and presumidly others) feel smart. 💡
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Relequestual is the current wording an improvement or not? If so, I can work to tweak it more. If not, I'd rather not spend a ton of time improving this new version- we can leave it as it was.
jsonschema-core.xml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, you don't like infinite asides?
It's a HUGE improvement! Bravo!
@Relequestual I did not add a step-by-step listing (I tried that originally but it got rather hard to do precisely because of too many conditionals). However, I added a paragraph in the intro section before the link to the example that informally summarizes how it works. Does that help?
5a99cdb to
37f0f99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on board.
I think it's a compelx issue to express, but the example grounds the explanation sufficiently to make it clear, at least to me.
I have no concerns over the potential to misinterpret what's defined.
This fills in a bit of a conceptual gap, and I think will help folks understand the "$id" / "$anchor" split. It also clarified some thinking on "$recursiveAnchor" for me which will be followed up on in future commits.
This re-casts the behavior of "$recursiveAnchor" in terms of base URIs, and now this whole section is much easier to explain. The examples (or rather, an updated example) will be added in the next commit.
37f0f99 to
1e1e79f
Compare
So.. I know I said I was done, but... I went to add a bit about
$idand$anchorbeing identification keywords (alongside assertions, annotations, applicators, and reserved locations), because it seemed like it might fix a conceptual gap and help folks understand what we're doing with the split, and then I realized that$recursiveAnchorfit into this category if we think about it as dynamically setting the base URI for resolving$recursiveRef. So that's the first commit.This led to a much more straightforward way of explaining how these keywords work, so I reworked that whole section. I feel like with the new presentation, the elaborate justification is not required. That's the second commit (including removing the old example).
The third commit is the new example, explained in terms of the new presentation of how the keywords work. It's down in the Appendixes with the other big example sections.