Closed tbmreza closed 1 year ago
Thanks you for the MR! Unfortunately I don't think it correctly implements the spec either. KDL identifiers are only concerned with code points, not grapheme clusters:
A bare Identifier is composed of any Unicode codepoint other than [decimal digits or non-identifier characters], followed by any number of Unicode codepoints other than non-identifier characters, [where a non-identifier character is]:
- Any codepoint with hexadecimal value
0x20
or below.- Any codepoint with hexadecimal value higher than
0x10FFFF
.- Any of
\/(){}<>;[]=,"
Additionally, KDL identifiers can't contain any whitespace, though that's not called out explicitly by the spec (https://github.com/kdl-org/kdl/issues/188)
whoops.
I'll update my PR as soon as I can 🐱💻
@Lucretiel Ready for review.
Thanks for the update, will review during the next stream (probably Monday)
Sorry to say I totally lost track of this pull request and ended up implementing it myself. I used a similar design, but taking advantage of Chars::as_str
to convert the chars iterator back into a string. Thanks you for the pull request, though!
The tests pass. What remains to be done for me is to figure out which error kinds to make.
Please have a look 🤓.