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

Fix patterns of provide_context gets panicked #37

Merged
merged 1 commit into from
Mar 19, 2018

Conversation

jmuk
Copy link
Contributor

@jmuk jmuk commented Mar 13, 2018

This PR fixes two cases where provide_context panics even though the previous call of is_boundary returns a PreContext error.

The primary cause is provide_context assumes its state as Regional or Emoji; otherwise it gets panicked with 'invalid state!'
However:

  • if the chunk starts with the cursor position, it requires pre-context for cat_before regardless of its state (and that's necessary for some cases indeed). so provide_context should fill cat_before in such case.
  • if both cat_before and cat_after are RIS and the boundary is undecided by the chunk, it requires pre-context but it does not set its state to Regional. This is done by setting state within handle_regional.

This PR fixes two cases where provide_context panics even though
the previous call of `is_boundary` returns a PreContext error.

The primary cause is `provide_context` assumes its state as
Regional or Emoji; otherwise it gets panicked with 'invalid state!'
However:
- if the chunk starts with the cursor position, it requires
  pre-context for `cat_before` (and that's necessary for some
  cases indeed). so provide_context should fill `cat_before`
  in such case.
- if both `cat_before` and `cat_after` are RIS and the boundary
  is undecided by the chunk, it requires pre-context but it does
  not set its state to Regional. This is done by setting state
  within `handle_regional`.
Copy link
Contributor

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

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

These changes look good to me. Thanks to jmuk for testing more thoroughly, clearly that's something I should have done.

@Manishearth Manishearth merged commit c3db7a7 into unicode-rs:master Mar 19, 2018
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