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

Feat/unicode grapheme support #6

Closed
wants to merge 2 commits into from

Conversation

arlyon
Copy link

@arlyon arlyon commented Apr 13, 2023

This closes #5. The PR is split into two commits. The first is kinda a precursor. I had a whole load of tests breaking to begin with which were tough to debug so now rather than a handful of tests with assertions, we now use test_case which exposes pass/fail for all 1200. The second PR is the 'meat'. It overrides 3 functions based on a feature flag.

To preserve performance, the non-unicode version still uses u8 rather than slices, though I feel like the perf overhead wouldn't be too large...

Note that rust requires tests to have a unique name so I have had to manually disambiguate any clashes (over 300 🥴). Some may be pretty crappy. In the process I have also removed identical tests.

@arlyon arlyon force-pushed the feat/unicode-grapheme-support branch from 809fb20 to 8f5f40b Compare April 13, 2023 15:26
@arlyon arlyon force-pushed the feat/unicode-grapheme-support branch from 8f5f40b to 2c31fed Compare April 13, 2023 15:43
arlyon added a commit to vercel/turborepo that referenced this pull request Apr 18, 2023
### Description

I have a PR open with glob-match to add unicode grapheme support. This
PR vendors the dependency until that lands
(devongovett/glob-match#6)

### Testing Instructions

`cargo test`

<!--
  Give a quick description of steps to test your changes.
-->

---------

Co-authored-by: Chris Olszewski <[email protected]>
NicholasLYang pushed a commit to NicholasLYang/turbo that referenced this pull request Apr 21, 2023
### Description

I have a PR open with glob-match to add unicode grapheme support. This
PR vendors the dependency until that lands
(devongovett/glob-match#6)

### Testing Instructions

`cargo test`

<!--
  Give a quick description of steps to test your changes.
-->

---------

Co-authored-by: Chris Olszewski <[email protected]>
@arlyon
Copy link
Author

arlyon commented Jan 4, 2024

Closing this since it has stalled

@arlyon arlyon closed this Jan 4, 2024
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
### Description

I have a PR open with glob-match to add unicode grapheme support. This
PR vendors the dependency until that lands
(devongovett/glob-match#6)

### Testing Instructions

`cargo test`

<!--
  Give a quick description of steps to test your changes.
-->

---------

Co-authored-by: Chris Olszewski <[email protected]>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
### Description

I have a PR open with glob-match to add unicode grapheme support. This
PR vendors the dependency until that lands
(devongovett/glob-match#6)

### Testing Instructions

`cargo test`

<!--
  Give a quick description of steps to test your changes.
-->

---------

Co-authored-by: Chris Olszewski <[email protected]>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
### Description

I have a PR open with glob-match to add unicode grapheme support. This
PR vendors the dependency until that lands
(devongovett/glob-match#6)

### Testing Instructions

`cargo test`

<!--
  Give a quick description of steps to test your changes.
-->

---------

Co-authored-by: Chris Olszewski <[email protected]>
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.

Unicode characters vs codepoints
1 participant