-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add IDL for LanguageMap
#88
Comments
Commenting on: aphillips@9624fbe I'm quite confused, because it seems like the language map examples are not consistent with each other or the
Could you help me understand how these definitions and examples are meant to relate? The way I interpret the intent of the prose descriptions, Example 4 matches what I would expect. I think the map syntax best expresses the intent of a collection of key-value pairs - and is easiest to work with as a developer - and it doesn't seem useful to use an explicit sequence structure for implementation efficiency, if that is the concern. Maps and sequences most likely take the same time to parse or search in anyway: in JSON, CBOR and XML a plain linear search is needed since the items don't describe their serialization length, while in ASN.1 DER the parser can "skip ahead" in a map just as easily as in a sequence. So without knowledge of any other concerns that went into this design, my expectation for a typedef record<DOMString, LanguageEntry> LanguageMap;
// Or alternatively:
typedef DOMString LanguageTag;
typedef record<LanguageTag, LanguageEntry> LanguageMap; Either of these would neatly match Example 4 and be easy to work with as a developer. |
@emlun Thanks for the comments. The IDL for Example 14 (and nearby friends) is definitely broken, wrt being valid JSON. I'll fix that also while making the necessary changes. |
Thanks @aphillips! The current design looks good to me. I spotted a few more minor issues:
|
It's a bug in Respec. Respec reports an error because
Fixed the first, replaced the @emlun Do you not have review permission on the PR? |
Oh! I didn't realize there was a PR. All I'd seen was the link to this issue from w3c/webauthn#2151, and #89 hasn't shown up in the activity feed in this thread. I'll post in the PR if I find anything else, but I think I'm done with my review for now. |
P.S. I added you to the acknowledgements. Appreciate the help! |
Address #88: Add WebIDL for LanguageMap, LanguageRecord, LanguageEntry and clean up appendix
Called out by Webauthn in w3c/webauthn#2151
The text was updated successfully, but these errors were encountered: