Skip to content
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

Fix links, remove as many anchor definitions as possible #1270

Merged
merged 5 commits into from
Feb 28, 2023

Conversation

tabatkins
Copy link
Contributor

@tabatkins tabatkins commented Feb 21, 2023

This is purely an editorial change. It fixes the link errors the current spec has (around Memory and Memory/buffer), and additionally removes a large number of the explicit anchors defined in the spec. For the most part these removals were simple; in a few places I fixed the linking syntax down in the spec body as well.

The abstract-op anchors left behind just aren't being detected by WebRef; I opened an issue about them. When that's fixed we should be able to remove that entire block.

A number of the remaining anchors are things that should probably be fixed in ECMAScript, but I don't have bandwidth to do that right now. I haven't done a full accounting of the remaining anchor values; some of them might also be removable right now.


Preview | Diff

@tabatkins
Copy link
Contributor Author

Oh hm, it's failing due to a badly-encoded URL, which I think is a Bikeshed bug I noticed before. I'll go investigate.

@tabatkins
Copy link
Contributor Author

Nah not a Bikeshed bug, it's a data source bug. Reported at w3c/webref#889

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Wow, this looks lovely.

index.bs Outdated
spec: ecma-262; type: dfn; for: /; text: internal method
spec: ecma-262; type: dfn; for: /; text: internal slot
spec: ecma-262; type: dfn; for: /; text:realm
spec:wasm-js-api-2;
Copy link
Member

@annevk annevk Feb 22, 2023

Choose a reason for hiding this comment

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

No variant without a version? cc @Ms2ger

This is something that needs to be fixed as this gives us two nearly identical references.

Copy link
Member

Choose a reason for hiding this comment

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

Happy to do whatever you tell me here :)

Note WebAssembly/spec#1611 , btw, which is why this xref is listed explicitly here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it'll be fixed separately, but in the meantime this link-default fixes the error locally.

[=ECMA-262 Ordinary object internal methods and internal slots=], and if the
object is a [=function object=], [=ECMA-262 Built-in function objects=].
[[ecmascript#sec-ordinary-object-internal-methods-and-internal-slots]], and if the
object is a [=function object=], [[ecmascript#sec-built-in-function-objects]].
Copy link
Member

Choose a reason for hiding this comment

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

This is wonderful. Thanks!

index.bs Outdated
@@ -7655,7 +7587,7 @@ values are represented by ECMAScript Object values.
An ECMAScript value |V| is [=converted to an IDL value|converted=]
to an IDL {{object}} value by running the following algorithm:

1. If <a abstract-op>Type</a>(|V|) is not Object, then [=ECMAScript/throw=] a {{ECMAScript/TypeError}}.
1. If <a abstract-op>Type</a>(|V|) is not Object, then [=ECMAScript/throw=] a {{TypeError}}.
Copy link
Member

Choose a reason for hiding this comment

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

This is another name clash between ES and Web IDL. Previously it linked to ECMAScript, but now it goes to Web IDL. It's mostly fine since Web IDL's {{TypeError}} is basically a redirect to ES. But since these algorithm steps are written in the style of ECMAScript (including the throw reference), I'd still prefer to use the ECMAScript link if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhh, I missed that distinction, indeed. That can be fixed. (Using <l spec=ecmascript>{{TypeError}}</l>.)

index.bs Outdated
Comment on lines 5089 to 5096
Note: Don't confuse the "{{SyntaxError!!exception}}" {{DOMException}} defined here
with ECMAScript's {{ECMAScript/SyntaxError}}.
with ECMAScript's {{SyntaxError}}.
"{{SyntaxError!!exception}}" {{DOMException}} is used to report parsing errors in web APIs,
for example when parsing selectors,
while the ECMAScript {{ECMAScript/SyntaxError}} is reserved for the ECMAScript parser.
while the ECMAScript {{SyntaxError}} is reserved for the ECMAScript parser.
To help disambiguate this further,
always favor the "{{SyntaxError!!exception}}" {{DOMException}} notation
over just using {{SyntaxError!!exception}} to refer to the {{DOMException}}. [[DOM]]
Copy link
Member

Choose a reason for hiding this comment

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

The new {{SyntaxError}} references are incorrect: it goes to the unrelated "SyntaxError" DOMException in Web IDL rather than ES. In fact, this paragraph explains exactly how they are different.

@domenic
Copy link
Member

domenic commented Feb 27, 2023

Looks like the webref data source bug is fixed, so this is ready for @tabatkins to fix up the exception references and then we should be good to go.

@tabatkins
Copy link
Contributor Author

All right, the TypeError and SyntaxError refs are fixed, and now that WebRef has fixed a lot of the missing dfns, I've done another pass thru the anchors and removed every single one that we no longer need. I specifically confirmed for all of these new removals that they didn't change any behavior; two or three now link more directly to a term in the first paragraph of a section, rather than the section itself, but I figured that was fine. The rest all link to the same anchors as before (just using the multipage version of the spec instead).

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

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

Thanks so much! I looked through the changes and everything looks good to me.

There were a few TypeError links that changed from ES to Web IDL, but I think they should've been IDL in the first place, so thanks for fixing those also.

@TimothyGu TimothyGu merged commit 50afeeb into whatwg:main Feb 28, 2023
@tabatkins tabatkins deleted the cleanup-links branch February 28, 2023 20:55
rakuco added a commit to rakuco/webidl that referenced this pull request Aug 14, 2023
PR whatwg#1270's substitutions ended up turning some occurrences of

    {{ECMAScript/TypeError}}

into

    <l spec=ecmascript>{{TypeError}}</l>}

which a trailing "}" character at the end.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants