Skip to content

Add "anchor" LDO keyword. #352

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

Merged
merged 2 commits into from
Sep 2, 2017
Merged

Conversation

handrews
Copy link
Contributor

Addresses the simpler half of #140.

This corresponds directly to the RFC 5988 "anchor" keyword.

I'm not sure whether all of the talk of the context URI and whether it is possible
to construct one belongs where I put it in the "rel" section or whether it should
be moved to "anchor", or a bit of each.

I'm also not sure if there needs to be more explanation of "anchor". Since it is
adapted from RFC 5988 I went with less explanation.

This corresponds directly to the RFC 5988 "anchor" keyword.
@handrews handrews added this to the draft-07 (wright-*-02) milestone Aug 20, 2017
@dlax
Copy link
Member

dlax commented Aug 20, 2017

I'm not sure whether all of the talk of the context URI and whether it is possible
to construct one belongs where I put it in the "rel" section or whether it should
be moved to "anchor", or a bit of each.

I assume you're referring to the "Depending on the media type of the instance, it it may or may not be possible to assign a URI to the exact default context resource" paragraph. I'm not sure it's needed at all for the simple case (URI) at stake here. Could you explain why you added it?
(Nit: having this in a dedicated commit would help review and understanding, as far as I am concerned.)

I'm also not sure if there needs to be more explanation of "anchor". Since it is
adapted from RFC 5988 I went with less explanation.

Looks enough to me.

@handrews
Copy link
Contributor Author

@dlax it's not in a dedicated commit because it's part of this logical change. I would not make one without the other. Without "anchor" there is no reason to think about what the context URI is.

As a general rule, I keep changes that interact with each other together in the same commit. I would not merge these separately.

@handrews
Copy link
Contributor Author

I assume you're referring to the "Depending on the media type of the instance, it it may or may not be possible to assign a URI to the exact default context resource" paragraph. I'm not sure it's needed at all for the simple case (URI) at stake here. Could you explain why you added it?

I thought I had answered this but apparently I didn't (or at least not very clearly). My apologies.

I included that because:

1.) not being able to produce a URI for the context resource feels strange to me, and therefore worth acknowledging. Without it, I feel fairly certain that someone would file an issue requesting clarification.

2.) The fact that we can't guarantee a context URI is due to the split nature of JSON / JSON Schema hypermedia documents. There are two resources involved: the schema and the instance (even if you never fetch the schema from a remote location). Most hypermedia documents involve one media type, and can therefore declare a fragment syntax to fulfill all use cases.

JSON Schema needs to work with instances that are plain application/json which has no such syntax. But people often think that the JSON Schema pointer fragment syntax can be applied to plain JSON. While it would seem to make sense to use it that, way, it is in fact explicitly forbidden by RFC 6901:

Note that a given media type needs to specify JSON Pointer as its
fragment identifier syntax explicitly (usually, in its registration
[RFC6838]). That is, just because a document is JSON does not imply
that JSON Pointer can be used as its fragment identifier syntax. In
particular, the fragment identifier syntax for application/json is
not JSON Pointer.

(added emphasis mine)

Many people (myself included once upon a time, before I understood media types and URI fragments as thoroughly as I do now) find this very counter-intuitive. I want to avoid widespread incorrect implementations, so I documented this in the spec. It's just too surprisng to ignore.

This should also help clarify the utility of the application/instance+json proposal, although discussion of that should stay over in #351. But having an (optional) instance media type declared by this spec would allow people to reliably use a properly registered standard fragment syntax. Is that worth making a new media type for? I don't know yet, discuss it in #351 :-)

@dlax
Copy link
Member

dlax commented Aug 22, 2017

@handrews Thanks for your detailed clarification. I agree it's worth mentioning.

@handrews
Copy link
Contributor Author

handrews commented Sep 1, 2017

Last call: as of tomorrow this will have been up for 14 days and I will merge. It is a near-universal link serialization feature and no one has really objected at any point (just discussed how to handle the non-URI-identifiable cases, which is independent of this keyword).

@handrews handrews merged commit e088bff into json-schema-org:master Sep 2, 2017
@handrews handrews deleted the link-anchor branch September 2, 2017 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants