Skip to content

Fix $anchor plain name format to match XML's NCName production #916

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
May 12, 2020

Conversation

ssilverman
Copy link
Member

This uses the US-ASCII part of the production.
See: https://www.w3.org/TR/2006/REC-xml-names11-20060816/#NT-NCName

@ssilverman ssilverman force-pushed the shawn/anchor-format branch 3 times, most recently from 953178b to 80ad9de Compare May 9, 2020 22:13
@handrews handrews requested a review from awwright May 9, 2020 22:32
@handrews
Copy link
Contributor

handrews commented May 9, 2020

@awwright I think you're the only person who would remember why the existing rules were done the way they were. If you are OK with this, I'm OK with this.

In section 5: Fragments, we reference the W3C best practices document, which at one point talks about the NCName production in the context of plain name fragments.

Whatever we do, section 5 and the section for $anchor need to agree.

Copy link
Member

@awwright awwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compatibility with XML and ASCII (because of the 7-bit nature of URIs, at least until we decide to commit to IRIs) were the only considerations, iirc.

For reference:

Name ::= NameStartChar (NameChar)*
NameStartChar ::= ":" | [A-Z] | "_" | [a-z] | [#xC0-#xD6] | [#xD8-#xF6] | [#xF8-#x2FF] | [#x370-#x37D] | [#x37F-#x1FFF] | [#x200C-#x200D] | [#x2070-#x218F] | [#x2C00-#x2FEF] | [#x3001-#xD7FF] | [#xF900-#xFDCF] | [#xFDF0-#xFFFD] | [#x10000-#xEFFFF]
NameChar ::= NameStartChar | "-" | "." | [0-9] | #xB7 | [#x0300-#x036F] | [#x203F-#x2040]

@ssilverman
Copy link
Member Author

ssilverman commented May 10, 2020

Just to be certain, the formatting and use of eref is okay? I'm thinking of instead wrapping that around some text. Do you guys have a style preference?

Update: On second thought, I like it how it is. I just force pushed, but I only added a period to end the sentence.

@ssilverman ssilverman force-pushed the shawn/anchor-format branch from 80ad9de to f8a6a29 Compare May 10, 2020 06:18
@ssilverman ssilverman requested a review from awwright May 10, 2020 06:20
@handrews
Copy link
Contributor

@ssilverman I'd make an <xref...>NCName production</xref> for better readability. I can't remember if that's already declared as an external document at the top or and bottom, but if not there are other W3C specs referenced- you'll figure it out :-)

You can't include an anchor in that reference, but "NCName production" will be enough for anyone to locate it within the spec.

@ssilverman
Copy link
Member Author

Updated to xref.

@handrews
Copy link
Contributor

@ssilverman you're getting a build error through Travis. I think you need to add a corresponding <!ENTITY declaration at the top. xrefs to external specifications require both that and an entry under references.

@ssilverman ssilverman force-pushed the shawn/anchor-format branch from 889e510 to fab4f8c Compare May 12, 2020 22:11
@ssilverman
Copy link
Member Author

ssilverman commented May 12, 2020

I added a missing "date" element; that seems to be what the DTD wanted. I also updated the target link because if the date is needed, then we need a link having a date that matches, not future revisions.

@handrews handrews merged commit 2232f5b into json-schema-org:master May 12, 2020
@ssilverman ssilverman deleted the shawn/anchor-format branch May 20, 2020 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants