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

Calculate the size of Text in UTF-16 code units #165

Merged
merged 1 commit into from
Mar 27, 2021
Merged

Conversation

hackerwins
Copy link
Member

@hackerwins hackerwins commented Mar 24, 2021

What this PR does / why we need it:

External editors often don't consider Grapheme Clusters, so we calculate the
size of Text in the same way as in JavaScript.

For more details:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split

Which issue(s) this PR fixes:

Fixes #110

Special notes for your reviewer:

yorkie-team/yorkie-js-sdk#167 (comment)

Does this PR introduce a user-facing change?:


Additional documentation:


Checklist:

  • Added relevant tests or not required
  • Didn't break anything

External editors often don't consider Grapheme Clusters, so we calculate the
size of Text in the same way as in JavaScript.

For more details:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split
@codecov
Copy link

codecov bot commented Mar 24, 2021

Codecov Report

Merging #165 (e1feaa8) into main (1c7ba76) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #165      +/-   ##
==========================================
+ Coverage   59.34%   59.37%   +0.02%     
==========================================
  Files          37       37              
  Lines        3289     3291       +2     
==========================================
+ Hits         1952     1954       +2     
  Misses       1151     1151              
  Partials      186      186              
Impacted Files Coverage Δ
pkg/document/json/rich_text.go 65.59% <100.00%> (+0.37%) ⬆️
pkg/document/json/text.go 53.62% <100.00%> (+0.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c7ba76...e1feaa8. Read the comment docs.

Copy link
Member

@mojosoeun mojosoeun left a comment

Choose a reason for hiding this comment

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

LGTM!

@hackerwins hackerwins merged commit 934e811 into main Mar 27, 2021
@hackerwins hackerwins deleted the text-size-utf16 branch March 27, 2021 03:07
jeonjonghyeok pushed a commit to jeonjonghyeok/yorkie that referenced this pull request Aug 4, 2022
External editors often don't consider Grapheme Clusters, so we calculate the
size of Text in the same way as in JavaScript.

For more details:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Got 'panic: offset should be less than or equal to length' when inserting emoji
4 participants