UTF-8 support #49
Merged
UTF-8 support #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
We could still use the patchwork library you know. |
colinodell
added
the
spec compliance
|
I looked into patchwork/utf8 as one possibility, but it seems to fallback to |
Spec 0.14 introduced the need to peek() for unicode whitespace: http://spec.commonmark.org/0.14/#right-facing-delimiter-run The Cursor therefore cannot be ignorant to multi-byte encodings.
colinodell
force-pushed
the
utf8-support
branch
from
dcafbfc
to
8c9d004
Compare
Merged
|
Requiring the extension is probably the best way to go then. :) |
|
Do you guys know if mbstring is that widespread? I personally have no idea |
|
I tried researching that but wasn't able to find any conclusive information. It does seem to be available in the most-popular Linux distros though:
|
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Spec version 0.14 enhanced some of the rules regarding emphasis parsing. It's now necessary to peek around at characters in a Unicode-aware way.
This introduces a major problem - the
Cursorclass is not currently Unicode-aware. For example, it thinks[Толпой]is 14 characters long instead of 8. This was fine for our purposes since we never needed to address Unicode characters by position.So the new challenge is that given a
Cursorin some position, we need to accurately obtain neighboring Unicode characters. Our$this->line[$this->position]trick won't work as it will only obtain a single byte instead of the whole character. The proper solution would require theCursorbecoming Unicode-aware.iconvandmbstringboth seemed to be strong candidates for this task, so I implemented both and benchmarked them.iconvresulted in a 1000% performance penalty, whereasmbstringrepresented a 26% drop, so I implemented the latter where needed.As much as I hate adding new dependencies and reducing performance, I do feel this is the correct approach. I definitely welcome any feedback, alternatives, or performance enhancements. I'll keep this open for a few days to gather feedback before accepting.
/cc @philsturgeon @cebe @GrahamCampbell @aleemb @dshafik