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

add @string.Index and refactor json parser #1631

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Yu-zh
Copy link
Collaborator

@Yu-zh Yu-zh commented Feb 10, 2025

The first commit adds the type @string.Index. Now there are three accessing methods for String:

  • nth_charcode(String, Int) -> Int // access n-th charcode in O(1) time
  • nth_codepoint(String, Int) -> Char // access n-th codepoint in O(n) time
  • codepoint_at(String, @string.Index) -> Char // access codepoint at given index in O(1) time
    We might need another one for completeness but I'm not sure about whether it's necessary:
  • charcode_at(String, @string.Index) -> Int // access charcode at given index in O(1) time
    The problem with the fourth method is that it needs another parameter shift~ : Int so that it can access the trailing surrogate.

And for taking view, there are two methods:

  • op_as_view(String, Int, Int) -> @string.View // taking the view by skipping a number of codepoints on both ends
  • as_view(String, @string.Index, @string.Index) -> @string.View // taking the view by just using the given index

The second commit shows how to use these methods to rewrite the json parser. The main benefit here is that there's no manual checking of surrogate pairs without sacrificing the efficiency.

@coveralls
Copy link
Collaborator

coveralls commented Feb 10, 2025

Pull Request Test Coverage Report for Build 5274

Details

  • 49 of 60 (81.67%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.06%) to 87.209%

Changes Missing Coverage Covered Lines Changed/Added Lines %
json/lex_number.mbt 1 2 50.0%
string/index.mbt 38 48 79.17%
Totals Coverage Status
Change from base Build 5273: -0.06%
Covered Lines: 5577
Relevant Lines: 6395

💛 - Coveralls

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.

3 participants