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

DictionaryMember default [] is rendered as None #1592

Closed
emlun opened this issue Jan 15, 2020 · 2 comments · Fixed by #1594
Closed

DictionaryMember default [] is rendered as None #1592

emlun opened this issue Jan 15, 2020 · 2 comments · Fixed by #1594

Comments

@emlun
Copy link
Contributor

emlun commented Jan 15, 2020

For example, see https://www.w3.org/TR/2019/WD-webauthn-2-20191126/#dom-publickeycredentialcreationoptions-excludecredentials, where this markup:

    sequence<PublicKeyCredentialDescriptor>      excludeCredentials = [];

is rendered as:

excludeCredentials, of type sequence<PublicKeyCredentialDescriptor>, defaulting to None

I'm new to the fine details of WebIDL and Bikeshed, but if I understand the specs correctly, [] is a valid default value and should probably be rendered as simply "[]". In that case, the root cause of the issue seems to be here:

https://github.com/tabatkins/bikeshed/blob/2a881f0ee698433d75c6a1555514334adb37d8c7/bikeshed/idl.py#L158-L160

which does not account for that construct.default.value will be None if the default value is [] or {}, in which case the object will have ._open and ._close set instead of .value.

Changing it to this seems to fix the issue, but it doesn't seem like the right solution:

                value = escapeAttr("{0}".format(construct.default.value if construct.default.value else (construct.default._open.symbol + construct.default._close.symbol)))

I'd be happy to contribute a fix if I can get a confirmation that my understanding seems correct, and perhaps some pointers to what might be a better solution. 🙂

@tabatkins
Copy link
Collaborator

Huh, interesting. Welp, it's definitely an error, but I'll have to look more into open/close to see what all that covers.

@emlun
Copy link
Contributor Author

emlun commented Jan 17, 2020

Ok, thanks for confirming! I'll look around some more and see if I can find something that seems like a good solution.

What I found is that the relevant values are set here in Default:

https://github.com/tabatkins/bikeshed/blob/2a881f0ee698433d75c6a1555514334adb37d8c7/bikeshed/widlparser/widlparser/productions.py#L1108-L1111

which is called from here in DictionaryMember:

https://github.com/tabatkins/bikeshed/blob/2a881f0ee698433d75c6a1555514334adb37d8c7/bikeshed/widlparser/widlparser/constructs.py#L1043-L1050

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 a pull request may close this issue.

2 participants