paste: permit the delimiter list to be empty #6714
Conversation
Also: refactored the delimiter processing logic
|
GNU testsuite comparison: |
andrewliebenow
force-pushed
the
paste-accept-empty-delimiter-list
branch
from
dcd0392
to
9c7381f
Compare
andrewliebenow
force-pushed
the
paste-accept-empty-delimiter-list
branch
from
976b924
to
d463af0
Compare
|
GNU testsuite comparison: |
andrewliebenow
force-pushed
the
paste-accept-empty-delimiter-list
branch
from
d463af0
to
e154510
Compare
|
GNU testsuite comparison: |
andrewliebenow
force-pushed
the
paste-accept-empty-delimiter-list
branch
2 times, most recently
from
f2928fc
to
a302d62
Compare
|
GNU testsuite comparison: |
src/uu/paste/src/paste.rs
Outdated
| // Is `map_err_context` correct here? | ||
| let file = File::open(path).map_err_context(String::new)?; |
There was a problem hiding this comment.
map_err_context itself looks correct to me, but not the String::new passed to it. I think it should be || name instead.
src/uu/paste/src/paste.rs
Outdated
| OneDelimiter { | ||
| delimiter: &'a [u8], | ||
| }, | ||
| MultipleDelimiters { | ||
| current_delimiter: &'a [u8], | ||
| delimiters: &'a [Box<[u8]>], | ||
| delimiters_iter: Cycle<Iter<'a, Box<[u8]>>>, | ||
| }, |
There was a problem hiding this comment.
Is it necessary to distinguish between a single delimiter and multiple delimiters? At least conceptually you could say there is no single delimiter, only an infinite list of delimiters.
There was a problem hiding this comment.
It just allows that case to be handled in a slightly cleaner/more efficient way, because we don't have to bother uselessly advancing a cycling iterator over a single element slice, and constantly recomputing the length of that single element. If you have a strong objection, I'm fine with removing it.
There was a problem hiding this comment.
No, I don't have a strong objection, it's fine.
andrewliebenow
force-pushed
the
paste-accept-empty-delimiter-list
branch
2 times, most recently
from
11af996
to
f096a21
Compare
|
GNU testsuite comparison: |
andrewliebenow
force-pushed
the
paste-accept-empty-delimiter-list
branch
from
8bb8a26
to
5a54253
Compare
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
| #[test] | ||
| fn test_non_utf8_input() { | ||
| // 0xC0 is not valid UTF-8 | ||
| const INPUT: &[u8] = b"Non-UTF-8 test: \xC0\x00\xC0.\n"; | ||
|
|
||
| new_ucmd!() | ||
| .pipe_in(INPUT) | ||
| .succeeds() | ||
| .stdout_only_bytes(INPUT); | ||
| } |
There was a problem hiding this comment.
Great improvement compared to the previous version with all its consts :)
There was a problem hiding this comment.
I had to discover "byte strings" (https://doc.rust-lang.org/stable/reference/tokens.html#characters-and-strings)!
|
Thanks! |
Also: refactored the delimiter processing logic