-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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 garbling when copying multibyte text via OSC 52 #7870
Conversation
New misspellings found, please review:
To accept these changes, run the following commands
✏️ Contributor please read thisBy default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
See the 🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! As far as the spellcheck is concerned, I'd update the L"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789\+/"
line in patterns.txt
to not have an L
at the start of it, and I'd add strchr
to apis.txt
result = L""; | ||
success = Base64::s_Decode(L"8J+RjfCfkY3wn4+78J+RjfCfj7zwn5GN8J+PvfCfkY3wn4++8J+RjfCfj78=", result); | ||
VERIFY_ARE_EQUAL(true, success); | ||
VERIFY_ARE_EQUAL(L"👍👍🏻👍🏼👍🏽👍🏾👍🏿", result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm mildly worried about putting the emoji in the file as literals like this - I know in the past we've had issues where someone's text editor saves a file with emoji as the wrong encoding which then end up breaking the tests later. I could be mistaken though, I'll let @DHowett or @miniksa refresh my memory if this is a bad idea or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naw, we've done this before. We'll survive!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for fixing this. I really appreciate it!
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
This commit adds a missing conversion utf8 to utf16 in decoding base64 for handling multibyte text in copying via OSC 52. ## Validation Steps Performed * automatically * Tests w/ multibyte characters * manually * case1 * Executed `printf "\x1b]52;;%s\x1b\\" "$(printf '👍👍🏻👍🏼👍🏽👍🏾👍🏿' | base64)"` * Verified `👍👍🏻👍🏼👍🏽👍🏾👍🏿` in my clipboard * case2 * Copied `👍👍🏻👍🏼👍🏽👍🏾👍🏿` by tmux 2.6 default copy function (OSC 52) * Verified `👍👍🏻👍🏼👍🏽👍🏾👍🏿` in my clipboard Closes #7819 (cherry picked from commit 743283e)
🎉 Handy links: |
🎉 Handy links: |
This commit adds a missing conversion utf8 to utf16 in decoding base64
for handling multibyte text in copying via OSC 52.
Validation Steps Performed
printf "\x1b]52;;%s\x1b\\" "$(printf '👍👍🏻👍🏼👍🏽👍🏾👍🏿' | base64)"
👍👍🏻👍🏼👍🏽👍🏾👍🏿
in my clipboard👍👍🏻👍🏼👍🏽👍🏾👍🏿
by tmux 2.6 default copy function (OSC 52)👍👍🏻👍🏼👍🏽👍🏾👍🏿
in my clipboardCloses #7819