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

Exception cleanup #250

Merged
merged 7 commits into from
Dec 13, 2016
Merged

Exception cleanup #250

merged 7 commits into from
Dec 13, 2016

Conversation

tobie
Copy link
Collaborator

@tobie tobie commented Dec 8, 2016

  • Don't define simple exception names in WebIDL.
  • Make Error exclusively for authors.
  • Fix syntax for throwing and creating DomExceptions.
  • Fix examples.

Closes #247.

NOTE: This doesn't address the distinction between #throw and #ecmascript-throw, which I think needs fixing before we merge this.

I'd be tempted not to export #ecmascript-throw and make everything use #throw instead. (Currently all simple exceptions in the spec reference #ecmascript-throw, which I don't think is the right thing to do.)
We'd then only use the text around #ecmascript-throw in the bindings to describe the behavior of all exceptions (or just simple ones?) in an ES environment. Thoughts?

NOTE 2: I opted for an "ExceptionName" {{DOMException}} syntax because it reads better, but I'm happy to switch it around if that makes more sense.


Preview | Diff w/ current ED | Diff w/ base

@tobie tobie requested a review from domenic December 8, 2016 16:38
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

I'd be tempted not to export #ecmascript-throw and make everything use #throw instead. (Currently all simple exceptions in the spec reference #ecmascript-throw, which I don't think is the right thing to do.)
We'd then only use the text around #ecmascript-throw in the bindings to describe the behavior of all exceptions (or just simple ones?) in an ES environment. Thoughts?

Sounds good to me. I kind of assumed that was how it worked already. I think the bindings only throw simple errors?

[=error name=].
[=Simple exceptions=] can be <dfn id="dfn-create-exception" for="exception" export>created</dfn>
by providing their [=error name=].
{{DOMException|DOMExceptions}}, by providing their [=error name=] followed by {{DOMException}}.
Copy link
Member

Choose a reason for hiding this comment

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

Grammar is a bit weird. I'd say "DOMExceptions can be created by providing..."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I remembered having a twitter thread about this grammar construction. Wasn't expecting @domenic to be the first reply, though. :D

I'll fix that.


[=Throw=] a TypeError.

[=Throw=] a <emu-val>TypeError</emu-val>.
Copy link
Member

Choose a reason for hiding this comment

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

So, I think in practice we want specifications to use {{TypeError}}, not <emu-val>TypeError</emu-val>. The emu-val is used for the "bindings" sections of specs like Web IDL and ES. So it's a bit confusing since this is the only place in the document that we'd use {{TypeError}}, but I think it makes sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So in that case, we need to define each simple exception somewhere (so back out of the changes right below #dfn-simple-exception).

Copy link
Member

Choose a reason for hiding this comment

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

I think they're already defined in ECMAScript.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. But where would we want specs to link to, however?

Copy link
Member

Choose a reason for hiding this comment

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

To ES!

Copy link
Member

Choose a reason for hiding this comment

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

Oh :(. Not sure what to do then if we want to avoid breaking lots of specs. I guess we could add silly redirect definitions. Or try to get a few choice entries into Shepherd somehow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So thinking about this a little more, I'm wondering why we'd treat simple exceptions differently than we treat buffer source types. (That is, we have a 1:1 IDL representation of these objects and specs link to the WebIDL one.) To be clear, I'm not objecting to the idea of merging bindings and WebIDL were it make sense, but shouldn't we have a clearer overall picture before we do so?

Copy link
Member

Choose a reason for hiding this comment

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

We should, since Wasm will require their own type I think.

Copy link
Member

Choose a reason for hiding this comment

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

Wasm, like assembly language, doesn't have exceptions at all, so this is pretty orthogonal.

I guess we do need definitions for the Web IDL types that correspond to the JS types.

Copy link
Collaborator Author

@tobie tobie Dec 9, 2016

Choose a reason for hiding this comment

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

Wasm, like assembly language, doesn't have exceptions at all, so this is pretty orthogonal.

Hadn't thought of this. Good point.

I guess we do need definitions for the Web IDL types that correspond to the JS types.

OK, so I'll move those back in, then.

* Don't define simple exception names in WebIDL.
* Make Error exclusively for authors.
* Fix syntax for throwing and creating DomExceptions.
* Fix examples.

Closes whatwg#247.
@domenic
Copy link
Member

domenic commented Dec 9, 2016

Latest version LGTM although I guess you still want to straighten out the various kinds of throw.

Cannot rename it to just throw with
an added for="ecmascript" attribute,
as that would have also modified WebIDL throw
for specs that already link to it.

Small clean-up that allows using shorthand.
@tobie
Copy link
Collaborator Author

tobie commented Dec 11, 2016

Latest version LGTM although I guess you still want to straighten out the various kinds of throw.

So I did some of that. Turns out it's hard to really straighten out, as adding a for attribute to "ecmascript throw" requires adding one to regular (WebIDL) "throw" too. In turn, I think this will break specs who are referencing WebIDL "throw" right now (afaik they would need to use something like [=/throw=] instead).

Alternatively, @tabatkins can prove me wrong and explain what I'd have to do to be able to use [=ecmascript/throw=] locally and export the other, regular (WebIDL) "throw" so folks can just refer to it as [=throw=] elsewhere.

In the meantime, I kept the old name and did some minor cosmetic changes, plus stopped exporting "ecmascript throw"

@tobie tobie merged commit c2d602f into whatwg:gh-pages Dec 13, 2016
@tobie tobie deleted the fix-247 branch December 13, 2016 09:46
@tabatkins
Copy link
Contributor

If you have both /throw and ecmascript/throw in your document, you'll locally have to use /throw everywhere. However, if you only export the /throw and not the other, then other specs will be able to just use throw. If you want to be extra-safe, you can do <dfn noexport lt="ECMAScript throw" local-lt="throw" for="ECMAScript">, so it won't even clash in the database as an unexported term.

@tobie
Copy link
Collaborator Author

tobie commented Dec 13, 2016

Thanks, @tabatkins, this is exactly what I needed.

tobie added a commit to tobie/webidl that referenced this pull request Dec 13, 2016
@tobie tobie mentioned this pull request Dec 13, 2016
tobie added a commit that referenced this pull request Dec 13, 2016
triple-underscore added a commit to triple-underscore/triple-underscore.github.io that referenced this pull request Dec 13, 2016
Meta: fix "throw" dfns (whatwg/webidl#256)
whatwg/webidl@7c1eab6
b3c76edd

Exception cleanup (whatwg/webidl#250)
whatwg/webidl@c2d602f
76b2950c

Editorial: clarify typedef aliasing limitations.
(whatwg/webidl#253)
whatwg/webidl@b4bc602
673c10b2

Disallow nullable Promise types.
(whatwg/webidl#248)
whatwg/webidl@7366419
cdaa8020

Add missing legacy error names.
(whatwg/webidl#251)
whatwg/webidl@c50ffc4
101c3b06

その他の編集
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.

None yet

4 participants