paste: permit the delimiter list to be empty by andrewliebenow · Pull Request #6714 · uutils/coreutils

andrewliebenow · 2024-09-19T13:52:16Z

Also: refactored the delimiter processing logic

github-actions · 2024-09-19T14:17:57Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
GNU test failed: tests/timeout/timeout. tests/timeout/timeout is passing on 'main'. Maybe you have to rebase?

tests/by-util/test_paste.rs

src/uu/paste/src/paste.rs

github-actions · 2024-09-22T18:17:25Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
GNU test failed: tests/timeout/timeout. tests/timeout/timeout is passing on 'main'. Maybe you have to rebase?

github-actions · 2024-09-23T10:56:43Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
GNU test failed: tests/timeout/timeout. tests/timeout/timeout is passing on 'main'. Maybe you have to rebase?

github-actions · 2024-09-27T20:04:03Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
GNU test failed: tests/timeout/timeout. tests/timeout/timeout is passing on 'main'. Maybe you have to rebase?

cakebaker · 2024-10-07T14:40:21Z

src/uu/paste/src/paste.rs

+            // Is `map_err_context` correct here?
+            let file = File::open(path).map_err_context(String::new)?;


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

cakebaker · 2024-10-07T15:42:51Z

src/uu/paste/src/paste.rs

+    OneDelimiter {
+        delimiter: &'a [u8],
+    },
+    MultipleDelimiters {
+        current_delimiter: &'a [u8],
+        delimiters: &'a [Box<[u8]>],
+        delimiters_iter: Cycle<Iter<'a, Box<[u8]>>>,
+    },


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.

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.

No, I don't have a strong objection, it's fine.

github-actions · 2024-10-08T12:08:37Z

GNU testsuite comparison:

Skip an intermittent issue tests/rm/rm1 (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/rm/rm2 is no longer failing!

github-actions · 2024-10-08T16:08:39Z

GNU testsuite comparison:

Skip an intermittent issue tests/rm/rm1 (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/rm/rm2 is no longer failing!

github-actions · 2024-10-09T06:06:16Z

GNU testsuite comparison:

Skip an intermittent issue tests/rm/rm1 (fails in this run but passes in the 'main' branch)
GNU test failed: tests/timeout/timeout. tests/timeout/timeout is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/rm/rm2 is no longer failing!

cakebaker · 2024-10-09T15:02:51Z

tests/by-util/test_paste.rs

+#[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);
+}


Great improvement compared to the previous version with all its consts :)

I had to discover "byte strings" (https://doc.rust-lang.org/stable/reference/tokens.html#characters-and-strings)!

cakebaker · 2024-10-10T13:36:38Z

Thanks!

paste: permit the delimiter list to be empty

37b085d

Also: refactored the delimiter processing logic

andrewliebenow force-pushed the paste-accept-empty-delimiter-list branch from dcd0392 to 9c7381f Compare September 20, 2024 21:58

Extract duplicated code into function

7a84883