Fix str.split bytes.split #1861
Conversation
4c749a2 to
c04c088
Compare
vm/src/obj/objbyteinner.rs
Outdated
| @@ -311,6 +309,8 @@ impl ByteInnerSplitlinesOptions { | |||
|
|
|||
| #[allow(clippy::len_without_is_empty)] | |||
| impl PyByteInner { | |||
| const ASCII_WHITESPACES: [u8; 6] = [0x20, 0x09, 0x0a, 0x0c, 0x0d, 0x0b]; | |||
There was a problem hiding this comment.
Can we use is_ascii_whitespace?
There was a problem hiding this comment.
find_byteset takes array, slice or vector instead of function. do you have any idea not to define this const?
vm/src/obj/objbyteinner.rs
Outdated
| Ok(elements) | ||
| } | ||
|
|
||
| fn rsplit_whitespace(&self, maxsplit: isize) -> Vec<Vec<u8>> { |
There was a problem hiding this comment.
It seems like there is a lot of code duplication of split_whitespace. Maybe we can create some sort of macro?
There was a problem hiding this comment.
Unlike split and rsplit, I don't see a good solution for _whitespaces to achieve enough readability. They have similar structure but lots of signs and variables are different.
vm/src/obj/objbyteinner.rs
Outdated
| splited | ||
| } | ||
|
|
||
| pub fn rsplit( |
There was a problem hiding this comment.
Again maybe we can share code with split?
There was a problem hiding this comment.
I think this is possible to share but split_whitespace looks to require too much effort
There was a problem hiding this comment.
@palaviv how do you think about this new revision?
which uses tons of bytes methods
Failing test depends on #1855