Add Unicode Bidi properties to the exported data.#6
Add Unicode Bidi properties to the exported data.#6cscott wants to merge 5 commits intonode-unicode:masterfrom
Conversation
|
Woah, that’s a lot of changes in a single PR :) I’ll have a look and leave some comments. Thanks! |
README.md
Outdated
There was a problem hiding this comment.
Any objection to making it bidi-mirroring instead?
There was a problem hiding this comment.
No objection. I was just trying to match the unicode property file name, which is BidiMirroring.txt.
Presumably you'd like the type value to be 'bidi-mirroring' and the datafile name to have the hyphen, etc, as well?
There was a problem hiding this comment.
Exactly. Just a nitpick of course, but I think it looks cleaner.
|
Awesome work! Thanks so much for this. I left some nitpicky comments. Let me know if you’re okay with making these small changes. |
There was a problem hiding this comment.
Wait, are these categories or properties?
You’ve added them to the properties here, which seems correct, but the code that generates the data seems to be in parse-categories.js. Am I missing something?
There was a problem hiding this comment.
As described in parse-categories.js, some of the 'categories' are actually properties (ANY, Assigned, ASCII, etc). These are more of the same.
These properties support the Unicode Bidirectional Algorithm.
|
I think I've addressed all your comments. I'm going to go back and read through again to make sure. |
|
I'm adding some test cases while I'm at it. :) hang on. |
|
Yay for more test cases! |
We also export an array directly mapping codepoints to properties where that makes sense: canonical category, bidi property, and the mirroring glyph.
|
Ok, I think I've got everything, and I added some test cases to boot. Let me know if I missed anything you asked me to change. |
There was a problem hiding this comment.
Why json: true here?
Edit: Ah, you probably wanted to wrap the result in quotes. jsesc has an option for that, though; jsesc(string, { wrap: true }) results in more compact output.
There was a problem hiding this comment.
@cscott Don’t bother, I’ve already done it in a follow-up commit. Thanks anyway!
These properties support the Unicode Bidirectional Algorithm. Ref. #6.
We also export an array directly mapping code points to properties where that makes sense: canonical category, bidi property, and the mirroring glyph. Ref. #6.
|
I’ve merged your changes; thanks! I left some new comments, as I’d like to discuss the directory structure / API a bit further before we publish the updated Unicode data modules to npm. Looking forward to hearing your thoughts! |
2 participants
No description provided.