forked from jroimartin/gocui
-
Notifications
You must be signed in to change notification settings - Fork 43
Proper unicode segmentation #87
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0b1db9d to
da05e71
Compare
…nctions Originally I had extracted free-standing functions because they are easier to test, but it's actually not hard at all to instantiate a TextArea and test its methods, and it gives us more flexibility to refactor the internals.
Now that we no longer test them, there's no longer any reason for the indirection.
ac98eb9 to
7c93cf8
Compare
go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix -test ./...
…ypeString Once at the end is enough.
Without any test names it is impossible to find out which test failed. Coming up with names is too much work; using indices is a good middle ground. Still not very convenient, but at least it is possible to find out which test case it was by counting.
They both have exactly the same logic, the only difference is that BackSpaceWord also deletes the characters between the old and new cursor position.
We bump it to the latest version of v2 for now; upgrading to v3 requires even more changes and will happen another time.
…egmentation This includes a complete reimplementation of the TextArea class. This is necessary for proper handling of multi-rune grapheme clusters such as 🏴 or ⚡️.
It uses a deprecated API, and would have to be changed to return a string rather than a rune. However, since there are no callers we might as well delete it.
I haven't measured if this actually makes a difference, but it seems like a sensible thing to do.
Lazygit has had its own copy of the SimpleEditor (for no reason, really), and it has been extended a bit over time. Backport those additions to gocui's SimpleEditor, so that we can use it in lazygit and get rid of the code duplication.
This is needed for being able to "type" emojis such as 🏴; such an emoji is a grapheme cluster that consists of a base rune (🏴 in this case), and a bunch of tag sequences that are not printable when they are used on their own. It is not really possible to type such an emoji (as far as I can tell), but you can copy/paste it, in which case the individual runes of this grapheme cluster will be passed to SimpleEditor one by one, and it is important that we add them all to the text area's content string.
7c93cf8 to
8abc274
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This bumps our tcell dependency, which recently switched to rivo/uniseg, which fixes several unicode rendering problems.
It then reimplements TextArea completely to take proper unicode segmentation into account, and fixes a lot of other issues related to that.