-
-
Notifications
You must be signed in to change notification settings - Fork 306
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
clarify that contentSchema holds a subschema and when/how it applies #1564
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
11216df
clarify that contentSchema holds a subschema and when/how it applies
gregsdennis 8e0466f
apply text wrap
gregsdennis 6342ec3
update note about processing contentSchema subschema in context
gregsdennis a90049e
Update specs/jsonschema-validation.md
gregsdennis f6276dc
contentSchema should not produce an annotation
gregsdennis 87f6201
Update specs/jsonschema-validation.md
gregsdennis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
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.
My initial reaction was that this doesn't make sense. The annotation is just the subschema. It's no longer in-place. It doesn't include the context of where it came from. So, how can it be evaluated in-place? Then it occurred to me that an annotation includes not just it's value, but also the schema location it came from and that location can be used to evaluate the
contentSchema
in-place. I don't think most readers are going to be knowledgeable enough to make that leap. This could use some clarification.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.
Does the footnote (
[^7]
) that follows not provide that clarity?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.
No, that's not the thing I'm saying needs to be clarified. We say that the annotation is the subschema. We say that the subschema shouldn't be evaluated out of context from where it appeared in the schema and we explain why in footnote 7. What we don't explain is given a subschema without its parent context, how is it even possible to evaluate it in context. The value of the annotation is just the subschema, not the context. We can't evaluate the subschema in context because we don't know the context in which it needs to be evaluated. I hope that makes sense this time.
Of course the solution is that the location of the annotation keyword in the schema is how you know the context, but that's not intuitive. This is the only annotation where the location of the keyword in the schema is useful or necessary to know. Usually, we only care about the value of the annotation. In this case, we need to know the value and the schema location of the annotation. Actually, when used correctly (in context), the value of the annotation is useless and it's the schema location that the user actually uses.
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.
This is incorrect. Annotation location has always been useful, especially in cases where you receive annotations from the same keyword in different locations, e.g. from
title
. The location allows the consumer to decide which (or both/all) it wants to use. This is Core, where annotations are defined.How would we not know the context? It's conveyed by the annotation location, which has always been defined to be a part of an annotation.
It's still not clear why you think that the existing text (including the lines following these) is insufficient. It's saying, "Don't just evaluate this annotation value as a schema because it may rely on things that exist externally to it. You probably need to evaluate it where it came from."
It's actually saying all of that, and then the footnote expands on that warning using an example.
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.
Yes, you're right. I acknowledged that in next paragraph. I was walking you through my thought process when I first read it and what I believe the vast majority of readers will be thinking when they read this section. If it took me a minute to make that connection, most readers won't make it at all. Yes, the concept is unambiguously documented elsewhere, but most readers won't have every detail of JSON Schema memorized and I think this is a pretty esoteric detail.