-
-
Notifications
You must be signed in to change notification settings - Fork 666
change webidl attribute to bitwise flag #4505
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
Conversation
+1. Would it be worth renaming |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec doesnt speak of Flags but extended attributes.
I think this is the wrong approach.
E.g. Clamp
https://webidl.spec.whatwg.org/#Clamp
The example would be basically:
const Clamp = Symbol('webidl.extended-attribute.Clamp')
webidl.converters.octet = function (V) {
// Clamped integer between 0 - 255
if (V[Clamp] === true) {
if (V > 255) {
return 255
}
if (V < 0) {
return 0
}
return V
}
// ToUint8 is basically modulo 256
return V & 255
}
const value = new Number(257)
console.log(webidl.converters.octet(value)) // should be '1'
but:
const value = new Number(257)
value[Clamp] = true
console.log(webidl.converters.octet(octet)) // should be '255'
So I think it is expected by webidl, that we set as it clearly states attributes to the values and not pass flags to webidl functions.
Your approach wouldn't work, in a number of ways that would take too long to explain (and I don't have any motivation to explain). This PR is a simple refactor of moving options as an object to options as a bitwise flag. |
} | ||
|
||
webidl.util.HasFlag = function (flags, attributes) { | ||
return typeof flags === 'number' && (flags & attributes) === attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need to check if flags is a number? cant we expect that we are smart enough to pass numbers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flags are optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we cant do it closer to the spec.
This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status