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

Support for hangul alt chars #196204

Merged
merged 26 commits into from Nov 27, 2023
Merged

Support for hangul alt chars #196204

merged 26 commits into from Nov 27, 2023

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Oct 22, 2023

Part of #196203

Recording 2023-10-22 at 07 48 14

TODO

  • Move into a new file (altChars.ts?) as this concept could be used elsewhere
  • Fixing lint by explicitly allowing unicode chars in this file as they make it much more readable/maintainable
  • Reduce string/array creation
  • Reduce repetition, for example when looking at char indexes in the character data
  • Consider optimizing/compressing the hangul character data
  • Add unit tests

@Tyriar Tyriar added this to the October 2023 milestone Oct 22, 2023
@Tyriar Tyriar self-assigned this Oct 22, 2023
@Tyriar Tyriar modified the milestones: October 2023, November 2023 Oct 22, 2023
@Tyriar
Copy link
Member Author

Tyriar commented Oct 22, 2023

@TylerLeonhardt what do you think about this new "alternate characters" concept? This could be adopted by other languages as needed. Here's what I plan on doing on the PR still but don't have time this iteration:

(moved to top)

@Tyriar Tyriar changed the title Start of support for hangul alt chars Support for hangul alt chars Oct 22, 2023
@TylerLeonhardt
Copy link
Member

I like the approach and think this is a good list of tasks. I wonder if there are other places where we can use this (settings editor, keybindings editor, etc)

@Tyriar
Copy link
Member Author

Tyriar commented Oct 24, 2023

Where ever this shared filtering code is used they'll get it for free, but I'm also planning on making it its own independent module if something else wants to integrate 👍

@BTCRob1993 BTCRob1993 mentioned this pull request Nov 4, 2023
@Tyriar Tyriar marked this pull request as ready for review November 23, 2023 12:44
@Tyriar
Copy link
Member Author

Tyriar commented Nov 23, 2023

@TylerLeonhardt this is pretty solid now, it doesn't work in go to file through which is one of the big use cases. I'm not sure if I can easily add it to go to file?

@Tyriar
Copy link
Member Author

Tyriar commented Nov 27, 2023

Note that this does not work in contiguous searches, so it must match from the start of the word. It's a hard optimization problem to get it to work for contiguous string checking too, at least without changing how that works to get off indexOf

auto-merge was automatically disabled November 27, 2023 20:37

Pull Request is not mergeable

@Tyriar Tyriar merged commit 04a6935 into main Nov 27, 2023
6 checks passed
@Tyriar Tyriar deleted the tyriar/hangul branch November 27, 2023 20:42
@github-actions github-actions bot locked and limited conversation to collaborators Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants