Fix utf8 length handling for shellwords #5738
Merged
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
Closed
|
I see it... thanks. |
the-mikedavis
added
C-bug
pascalkuthe
approved these changes
Feb 1, 2023
|
|
||
| #[test] | ||
| fn test_multibyte_at_end() { | ||
| assert_eq!(Shellwords::from("𒀀").parts(), &["𒀀"]); |
There was a problem hiding this comment.
I liked your examples in the PR description, let's add cases for them to make sure we don't have regressions in the future 👍
Suggested change
| assert_eq!(Shellwords::from("𒀀").parts(), &["𒀀"]); | |
| assert_eq!(Shellwords::from("𒀀").parts(), &["𒀀"]); | |
| assert_eq!(Shellwords::from(":sh echo 𒀀").parts(), &[":sh", "echo", "𒀀"]); | |
| assert_eq!(Shellwords::from(":sh echo hello𒀀").parts(), &[":sh", "echo", "hello𒀀"]); |
There was a problem hiding this comment.
I added the change as a separate commit as it is generally cleaner to review (as opposed to an amend). I can squash them if you want. What is the general preference for this project?
There was a problem hiding this comment.
reviewers generally squash themselves for small PRs like this if they find it necessary. I don't think it matters much either way,
trink
force-pushed
the
shellwords_utf8
branch
from
9bd8cef
to
99dfa62
Compare
If the last argument to shellwords ends in a multibyte utf8 character the entire argument will be dropped. e.g. `:sh echo test1 test2𒀀` will only output `test1` Add additional tests based on the code review feedback
the-mikedavis
force-pushed
the
shellwords_utf8
branch
from
99dfa62
to
fa28198
Compare
|
(Force pushed without changes to trigger CI. I'm not sure why it didn't run the first time 🤔) |
the-mikedavis
approved these changes
Feb 1, 2023
the-mikedavis
changed the title
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.
If the last argument to shellwords ends in a multibyte utf8 character the entire argument will be dropped.
e.g.
:sh echo test1 test2𒀀will only outputtest1