-
Notifications
You must be signed in to change notification settings - Fork 93
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
Define parse JSON from bytes #156
Conversation
Needed for Fetch, XMLHttpRequest, and probably Origin Policy, Manifest, and maybe more.
I wonder if it's worth pointing out that "web platform JSON" can start with a BOM. |
infra.bs
Outdated
@@ -893,6 +895,21 @@ For <a>pairs</a>, a slightly shorter literal syntax can be used, separating the | |||
as 200/`<code>OK</code>`. | |||
|
|||
|
|||
<h2 id=json>JSON</h2> | |||
|
|||
<p>To <dfn>parse JSON from bytes</dfn> given <var>bytes</var>, run these steps: |
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.
Add export=""
Thankfully eating up the BOM is sanctioned per https://tools.ietf.org/html/rfc7159#section-8.1 even though it's not part of the grammar so I think we're good. And per http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5399 it also matches implementations. |
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.
LGTM with the abstract-op change at least.
infra.bs
Outdated
@@ -25,6 +25,8 @@ Boilerplate: omit conformance, omit feedback-header, omit idl-index | |||
|
|||
<pre class="anchors"> | |||
urlPrefix: https://tc39.github.io/ecma262/; spec: ECMA-262; type: dfn | |||
text: %JSONParse%; url: sec-json.parse | |||
text: Call; url: sec-call |
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.
Call
should be abstract-op
, not dfn
I guess we don't have a good name for stand-alone functions, so we can leave that as a dfn, despite that causing casing issues in "terms referenced by this specification". /cc @tabatkins
infra.bs
Outdated
<li><p>Let <var>jsonText</var> be the result of running <a>UTF-8 decode</a> on <var>bytes</var>. | ||
|
||
<li> | ||
<p>Return ? <a>Call</a>(<a>%JSONParse%</a>, undefined, « <var>jsonText</var> »). |
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.
Should be <a abstract-op>
with the above changes, for Call.
Hmm, in the branch snapshot [ENCODING] is still marked as an informative reference, despite you using UTF-8 decode in normative text... a Bikeshed issue, perhaps? |
Still LGTM but kind of annoying we have to add the explicit [specname]s. I guess I never was clear on the rules for those anyway. |
I think the references section is solely populated from |
That's sort of the style that's been used forever, but I agree it's somewhat weird. We could consider alternatives, but probably best in a new issue. |
Hmm, there is indeed now one normative reference to Encoding in the spec, but it's still showing up in the Informative section (if I remove your manual normative reference). Some debugging statements shows that this is because it can't find the spec in the biblio db, but when I put the exact same autolink in a test document, it finds it just fine. This is... weird, to say the least. Further debugging required. |
Fixed now. |
File a bug to hook into this from manifest spec. Thanks for adding. |
Needed for Fetch, XMLHttpRequest, and probably Origin Policy, Manifest, and maybe more.
Preview | Diff