Improve word and file name completion with non-ASCII characters #946
Conversation
This file contains 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
mcepl
approved these changes
Aug 2, 2023
There was a problem hiding this comment.
Already included in https://git.sr.ht/~mcepl/vis and it seems to work just fine.
rnpnr
reviewed
Aug 10, 2023
vis-menu.c
Outdated
| valid = textvalidn(t, w-4); | ||
| if (valid < 0) | ||
| die("invalid UTF-8 sequence"); | ||
| for (i = MAX(valid, 0); i < tw; i++) buf[i] = '.'; |
There was a problem hiding this comment.
I'm not sure about calling die() here. It seems unnecessary.
|
There are definitely still some issues with handling of non-ASCII characters I'm thinking of applying it apart from the call to |
|
On August 10, 2023 5:59:08 PM GMT+02:00, Randy Palamar ***@***.***> wrote:
There are definitely still some issues with handling of non-ASCII characters
after applying this but overall I think this improves the situation.
I'm thinking of applying it apart from the call to `die()`. Any objections?
I'm also for removing the "die()" before applying. Thanks for picking up this PR!
…--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
Before we were not taking non-ascii characters into account properly. With this patch we still mix byte counts and "grapheme cluster" (i.e. complete glyphs that are rendered in a terminal cell) counts but the code should be less broken in the more common case now.
The '[:alnum:]' set does not include non-ascii text which results in the non-ascii text being replaced with newlines. Using the '[:blank:]' set with no complement flag fixes this issue.
rnpnr
force-pushed
the
try-to-preserve-codepoints
branch
from
976bf60
to
711447a
Compare
|
Applied, thanks for the patch! |
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.
These two commits fix this issue #941.
Note that
vis-menuis still not handling Unicode correctly (it's still not differentiating properly between bytes and "grapheme clusters", i.e. printable glyph-combinations, when calculating lengths). With these changes we are at least able to have a working support for non-ASCII characters in the simple case.