Skip to content
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

grapheme support #1478

Closed
wants to merge 5 commits into from
Closed

grapheme support #1478

wants to merge 5 commits into from

Conversation

jerch
Copy link
Member

@jerch jerch commented May 30, 2018

WIP - dont merge.

final += `export const SECOND: string = '${second}';\n`;
final += `// THIRD: codepoint >= 65536\n`;
final += `export const THIRD: string = '${third}';\n`;
require('fs').writeFileSync(path, final);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect the data file to ever change? If not I'd prefer to do a one time conversion and not include conversion scripts/data files in the repo

Copy link
Member Author

@jerch jerch May 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It changes at least with every major release and the consortium updates the spec in a yearly fashion. In 2 months we should see unicode v11.0.
The changes address mostly new chars and updates of drafted specs, it seems the older a char the less likely is an update.

I am not a fan of the code generation script thingy either. It may introduce several problems due to inexpected changes in the spec - like the region_indicator from 9.0 to 10.0 where they even gave up the previous-next rule, which will not work correctly until explicitly handled in some corresponding code. Maybe we should tag a supported unicode version and stick to it for some time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: The data generator could be parked in some dev module to avoid rewriting it once we decide to upgrade to a newer unicode version in future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xtermjs/node-grapheme-cluster-break?

@Tyriar Tyriar added the work-in-progress Do not merge label May 31, 2018
@Tyriar Tyriar added the reference A closed issue/pr that is expected to be useful later as a reference label Aug 31, 2018
@jerch
Copy link
Member Author

jerch commented Aug 31, 2018

Closing for now, needs a proper way of dealing with different unicode versions that has to come. See also #1470.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reference A closed issue/pr that is expected to be useful later as a reference work-in-progress Do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants