Skip to content

Editorial: Align with Web IDL specification - #200

Closed
autokagami wants to merge 1 commit into
immersive-web:masterfrom
autokagami:layers
Closed

Editorial: Align with Web IDL specification#200
autokagami wants to merge 1 commit into
immersive-web:masterfrom
autokagami:layers

Conversation

@autokagami

@autokagami autokagami commented Aug 18, 2020 •
edited by pr-preview Bot
Loading

Copy link
Copy Markdown

This is an automated pull request to align the spec with the latest Web IDL specification.

Currently the autofix might introduce some awkward code formatting, so please feel free to modify the formatting.

Please file an issue on https://github.com/saschanaz/webidl-updater/issues/new if you think this PR is invalid or should be enhanced.

The following is the validation messages from webidl2.js, which may help understanding this PR:

Validation error at line 7 in layers,16, inside `interface XRWebGLBinding -> operation createProjectionLayer -> argument init`:
                                          optional XRProjectionLayerInit init);
                                                                         ^ Optional dictionary arguments must have a default value of `{}`.

Validation error at line 9 in layers,16, inside `interface XRWebGLBinding -> operation createQuadLayer -> argument init`:
                              optional XRQuadLayerInit init);
                                                       ^ Optional dictionary arguments must have a default value of `{}`.

Validation error at line 11 in layers,16, inside `interface XRWebGLBinding -> operation createCylinderLayer -> argument init`:
                                      optional XRCylinderLayerInit init);
                                                                   ^ Optional dictionary arguments must have a default value of `{}`.

Validation error at line 13 in layers,16, inside `interface XRWebGLBinding -> operation createEquirectLayer -> argument init`:
                                      optional XREquirectLayerInit init);
                                                                   ^ Optional dictionary arguments must have a default value of `{}`.

Validation error at line 14 in layers,16, inside `interface XRWebGLBinding -> operation createCubeLayer -> argument init`:
  XRCubeLayer createCubeLayer(optional XRCubeLayerInit init);
                                                       ^ Optional dictionary arguments must have a default value of `{}`.

Preview | Diff

@cabanier

cabanier commented Aug 18, 2020

Copy link
Copy Markdown
Member

@saschanaz Please stop submitting PRs!

@cabanier cabanier closed this Aug 18, 2020
@saschanaz

saschanaz commented Aug 18, 2020

Copy link
Copy Markdown

@cabanier I think this one is legit, could you do a double check?

Sorry to make you busy 🙇‍♀️

@cabanier

cabanier commented Aug 18, 2020

Copy link
Copy Markdown
Member

@cabanier I think this one is legit, could you do a double check?

Sorry to make you busy 🙇‍♀️

There's a bikeshed issue that blocks me from removing the optional keyword. By adding = {} the spec now implies that there's a default initialization for the dictionaries.

@saschanaz

saschanaz commented Aug 18, 2020 •
edited
Loading

Copy link
Copy Markdown

You can't remove the keyword because the Web IDL spec requires it:

If the type of an argument is a dictionary type or a union type that has a dictionary type as one of its flattened member types, and that dictionary type and its ancestors have no required members, and the argument is either the final argument or is followed only by optional arguments, then the argument must be specified as optional and have a default value provided.

In this case the dictionary members in XRProjectionLayerInit are all optional so the argument must be optional too.

@cabanier

cabanier commented Aug 18, 2020

Copy link
Copy Markdown
Member

They're not optional.
All (except the first dictionary) inherit from xrlayerinit.

I agree the first one should have the ={}.

@saschanaz

saschanaz commented Aug 18, 2020

Copy link
Copy Markdown

Oops, you're right, so this is speced/bikeshed#1566. Sorry for bothering you 🤯

saschanaz added a commit to saschanaz/webidl-updater that referenced this pull request Feb 20, 2021
@saschanaz
chore(rewrite): remove webxrlayers from denylist
4b011e1 
immersive-web/layers#248 fixed it. Not sure why immersive-web/layers#200 was rejected then 🤷‍♀️
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@autokagami @cabanier @saschanaz