You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hello. 👋 For a project, I'm reviewing over a dozen color parsing libraries, all of which have various problems. Yours is one of the closest to correctness, only having this one issue. Nice!
The alpha is incorrectly being set to a default value when it is actually undefined. For example:
While this alpha can be inferred as 1, for tokenization and parsing, alpha is not set, and should not be included in the parsed results. By forcing a default alpha, it removes the ability to know what the actual input was and precisely convert it.
For example, should converting #00f to rgb color mode be rgb(0 0 255) or rgb(0 0 255 / 1)? It's indeterminate from the result of parse. While it is true that they are equivalent in an rgb color model, at a parsing level they are not equivalent. To be a perfect 1:1 conversion in a parsing scenario, the output result must be rgb(0 0 255), but there's no way to infer that from the parsing result.
Hello. 👋 For a project, I'm reviewing over a dozen color parsing libraries, all of which have various problems. Yours is one of the closest to correctness, only having this one issue. Nice!
The alpha is incorrectly being set to a default value when it is actually undefined. For example:
While this alpha can be inferred as 1, for tokenization and parsing, alpha is not set, and should not be included in the parsed results. By forcing a default alpha, it removes the ability to know what the actual input was and precisely convert it.
For example, should converting
#00f
torgb
color mode bergb(0 0 255)
orrgb(0 0 255 / 1)
? It's indeterminate from the result ofparse
. While it is true that they are equivalent in an rgb color model, at a parsing level they are not equivalent. To be a perfect 1:1 conversion in a parsing scenario, the output result must bergb(0 0 255)
, but there's no way to infer that from the parsing result.Expectation:
The text was updated successfully, but these errors were encountered: