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

Pull in utf8 support #27

Merged
merged 6 commits into from
Mar 17, 2024
Merged

Pull in utf8 support #27

merged 6 commits into from
Mar 17, 2024

Conversation

apainintheneck
Copy link
Owner

@apainintheneck apainintheneck commented Feb 27, 2024

This PR pulls in UTF8 support from the antirez/linenoise#187 branch as a part of #21. So far so good but I want to update docs and do some more testing before merging.

It only covers unicode version 13.0 but that's way better than what it was before. In all likelihood I won't need the additional unicode characters that were added in 14.0 and 15.0 but I should document how this was generated as best as possible so that I could technically move to that in the future.

Anyway, I'm very impressed so far.

TODO:

  • Document changes in extension changelog
  • Document process of generating the character widths
  • See about updating tests to account for this UTF8 support
  • Create an issue to find an alternative for expect that supports UTF-8 and runs on CI

This is the first batch of changes from an upstream PR that provides
UTF8 support. As a side effect, it means that we now will have the
correct support for escape sequences in single and multiline mode.

We previously merged in a smaller change which provided support
for single line mode with escape sequences by fixing the cursor
position but this is more comprehensive.

The next commit will add the UTF8 library.

Author: https://github.com/yhirose
PR: antirez/linenoise#187
This is the second half of the PR that I'm merging in from upstream.
It adds UTF8 13.0 support by including an extra library to get the
char width of those characters (the visual width that is). During
testing things seem to work quite well. I'm impressed.

Author: https://github.com/yhirose
PR: antirez/linenoise#187
@apainintheneck apainintheneck added the enhancement New feature or request label Feb 27, 2024
@apainintheneck
Copy link
Owner Author

It looks like the old default version of expect macOS doesn't really like UTF-8 strings and started choking on them locally when I tried to update those tests. The manual testing shows that it works as expected. Maybe it's time to consider on a deeper level if there is some better way of testing things here. It doesn't help that expect dies on CI because it only has access to a dumb terminal.

@apainintheneck
Copy link
Owner Author

This should be good enough for now. I'll create an issue about finding an alternative for expect that allows me to test UTF-8 strings and that runs on CI.

@apainintheneck apainintheneck marked this pull request as ready for review March 17, 2024 06:25
@apainintheneck
Copy link
Owner Author

@yhirose The original PR worked really well for adding UTF-8 support to Linenoise. Thanks!

@apainintheneck apainintheneck merged commit 0479e7b into main Mar 17, 2024
2 checks passed
@apainintheneck apainintheneck deleted the pull-in-utf8-support branch March 17, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant