tr: Accept non UTF-8 arguments for sets#6563
Conversation
a09e6f8 to
52d0431
Compare
|
GNU testsuite comparison: |
52d0431 to
aac483e
Compare
|
I'm having an issue here. To do this, I can either use The other solution is to use Is there another possible solution ? Which one shall we proceed with ? @tertsdiepraam |
|
GNU testsuite comparison: |
|
That's an issue indeed. It seems @sylvestre doesn't want to raise the MSRV, and I don't know enough on the topic to have an opinion. So let's not do that, for now. This isn't the first time that uutils needs to convert an OsString to bytes, so I suggest taking any existing implementation (e.g. in So in short I'm suggesting to use |
wat ? :) |
|
GNU testsuite comparison: |
7058659 to
5fbaeb6
Compare
|
I moved |
BenWiederhake
left a comment
There was a problem hiding this comment.
The changes look good, the test is fine, lovely!
CI is red for a reason, please fix those issues first:
- Missing "use" on Windows: https://github.com/uutils/coreutils/actions/runs/9955233780/job/27502537737?pr=6563
- Still too high MSRV: https://github.com/uutils/coreutils/actions/runs/9955233780/job/27502538603?pr=6563
I recommend doingrustup default 1.70or something like that, which should switch your toolchain down to version 1.70. Then you'll automatically notice any problems! - Remaining CI didn't even run because of that.
|
My bad, thanks for noticing |
68305d2 to
8ec48d6
Compare
BenWiederhake
left a comment
There was a problem hiding this comment.
Yup, thank you! Remaining CI failures are known issues: #6534 #6570 (both)
I hope I'm not being too nitpicky, but can you change the first commit's subject line to follow the convention a bit closer? I suggest:
cut: move os_string_as_bytes utility to uucore(mention util name)tr: accept non utf8 arguments for sets(lowercase first letter)
Thanks for fixing tr! :)
8ec48d6 to
dac38bb
Compare
Done ! 😁 |
Fixes #6343