From 9bbb58b746c41802278b0cba738eebbf21517cf7 Mon Sep 17 00:00:00 2001 From: akervald <113261029+akervald@users.noreply.github.com> Date: Fri, 27 Feb 2026 13:31:07 +0100 Subject: [PATCH] cut: fix -s flag for newline delimiter and improve performance - Fixed the -s flag incorrectly suppressing output when the delimiter is a newline. - Improved performance in cut_fields_newline_char_delim. - Updated tests to match GNU cut behavior for newline delimiters. --- src/uu/cut/benches/cut_bench.rs | 18 +++ src/uu/cut/src/cut.rs | 136 +++++++++++++++++++--- tests/by-util/test_cut.rs | 199 ++++++++++++++++++++++++++++++++ 3 files changed, 337 insertions(+), 16 deletions(-) diff --git a/src/uu/cut/benches/cut_bench.rs b/src/uu/cut/benches/cut_bench.rs index 997235f88..c0288a854 100644 --- a/src/uu/cut/benches/cut_bench.rs +++ b/src/uu/cut/benches/cut_bench.rs @@ -71,6 +71,24 @@ fn cut_fields_custom_delim(bencher: Bencher) { }); } +/// Benchmark cutting fields with newline delimiter +#[divan::bench] +fn cut_fields_newline_delim(bencher: Bencher) { + let mut data = Vec::new(); + for i in 0..100_000 { + let line = format!("field_content_number_{i}\n"); + data.extend_from_slice(line.as_bytes()); + } + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["-d", "\n", "-f", "1,3,5", file_path.to_str().unwrap()], + )); + }); +} + fn main() { divan::main(); } diff --git a/src/uu/cut/src/cut.rs b/src/uu/cut/src/cut.rs index 7819683f4..bc50cce62 100644 --- a/src/uu/cut/src/cut.rs +++ b/src/uu/cut/src/cut.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) delim sourcefiles +// spell-checker:ignore (ToDO) delim sourcefiles undelimited use bstr::io::BufReadExt; use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser}; @@ -254,35 +254,132 @@ fn cut_fields_implicit_out_delim( Ok(()) } -/// The input delimiter is identical to `newline_char` +/// Streams and filters fields where the record terminator and +/// field delimiter are the same character (specified by `newline_char`) fn cut_fields_newline_char_delim( reader: R, out: &mut W, ranges: &[Range], + only_delimited: bool, newline_char: u8, out_delim: &[u8], ) -> UResult<()> { - let buf_in = BufReader::new(reader); + let mut reader = BufReader::new(reader); + let mut line = Vec::new(); - let segments: Vec<_> = buf_in.split(newline_char).filter_map(Result::ok).collect(); - let mut print_delim = false; + // We start at 1 because 'cut' field indexing is 1-based + let mut current_field_idx = 1; + let mut first_field_printed = false; + let mut has_data = false; + let mut suppressed = false; - for &Range { low, high } in ranges { - for i in low..=high { - // "- 1" is necessary because fields start from 1 whereas a Vec starts from 0 - if let Some(segment) = segments.get(i - 1) { - if print_delim { - out.write_all(out_delim)?; + let mut range_idx = 0; + + loop { + line.clear(); + + let is_selected = range_idx < ranges.len() && current_field_idx >= ranges[range_idx].low; + let needs_data = is_selected || current_field_idx == 1; + + let mut has_processed_data = false; + + if needs_data { + // Standard read: copies bytes into `line` + loop { + let buf = reader.fill_buf()?; + if buf.is_empty() { + break; + } + + has_processed_data = true; + + if let Some(pos) = memchr::memchr(newline_char, buf) { + let amt = pos + 1; + line.extend_from_slice(&buf[..amt]); + reader.consume(amt); + + break; + } + let len = buf.len(); + line.extend_from_slice(buf); + reader.consume(len); + } + } else { + // Zero-allocation skip: scans the buffer and advances the cursor without copying + loop { + let buf = reader.fill_buf()?; + if buf.is_empty() { + break; // EOF + } + + has_processed_data = true; + + if let Some(pos) = memchr::memchr(newline_char, buf) { + let bytes_to_consume = pos + 1; + reader.consume(bytes_to_consume); + break; + } + + let len = buf.len(); + reader.consume(len); + } + } + + if !has_processed_data { + break; + } + has_data = true; + + // To comply with -s when the stream consists of only a single field. + if current_field_idx == 1 { + let is_eof_next = reader.fill_buf()?.is_empty(); + + if is_eof_next && line.last() != Some(&newline_char) { + if only_delimited { + suppressed = true; } else { - print_delim = true; + // GNU cut prints the whole line if no delimiter is found. + out.write_all(&line)?; } - out.write_all(segment.as_slice())?; - } else { break; } } + + if range_idx < ranges.len() && current_field_idx > ranges[range_idx].high { + range_idx += 1; + + // EARLY EXIT: If we've exhausted all ranges, stop reading the stream entirely. + if range_idx == ranges.len() { + break; + } + } + + // Check if the current field falls inside the current active range + let is_selected = range_idx < ranges.len() && current_field_idx >= ranges[range_idx].low; + + if is_selected { + if first_field_printed { + out.write_all(out_delim)?; + } + + let has_newline = line.last() == Some(&newline_char); + let content = if has_newline { + &line[..line.len() - 1] + } else { + &line[..] + }; + + out.write_all(content)?; + first_field_printed = true; + } + + current_field_idx += 1; } - out.write_all(&[newline_char])?; + + if has_data && !suppressed { + out.write_all(&[newline_char])?; + } + Ok(()) } @@ -297,7 +394,14 @@ fn cut_fields( match field_opts.delimiter { Delimiter::Slice(delim) if delim == [newline_char] => { let out_delim = opts.out_delimiter.unwrap_or(delim); - cut_fields_newline_char_delim(reader, out, ranges, newline_char, out_delim) + cut_fields_newline_char_delim( + reader, + out, + ranges, + field_opts.only_delimited, + newline_char, + out_delim, + ) } Delimiter::Slice(delim) => { let matcher = ExactMatcher::new(delim); diff --git a/tests/by-util/test_cut.rs b/tests/by-util/test_cut.rs index 79fb1488a..c3a512be9 100644 --- a/tests/by-util/test_cut.rs +++ b/tests/by-util/test_cut.rs @@ -301,6 +301,205 @@ fn test_newline_as_delimiter_with_output_delimiter() { .stdout_only_bytes("a:b\n"); } +#[test] +fn test_newline_as_delimiter_no_delimiter_suppressed() { + for param in ["-s", "--only-delimited", "--only-del"] { + new_ucmd!() + .args(&["-d", "\n", param, "-f", "1"]) + .pipe_in("abc") + .succeeds() + .no_output(); + } +} + +#[test] +fn test_newline_as_delimiter_found_not_suppressed() { + // Has an internal \n delimiter, so -s shouldn't suppress it + for param in ["-s", "--only-delimited", "--only-del"] { + new_ucmd!() + .args(&["-d", "\n", param, "-f", "1"]) + .pipe_in("abc\ndef\n") + .succeeds() + .stdout_only("abc\n"); + } +} + +#[test] +fn test_newline_as_delimiter_multiple_fields() { + // Check field selection when \n is the delimiter + new_ucmd!() + .args(&["-d", "\n", "-f", "2"]) + .pipe_in("abc\ndef\n") + .succeeds() + .stdout_only("def\n"); +} + +#[test] +fn test_newline_as_delimiter_double_newline() { + // Field 2 is the empty space between newlines + new_ucmd!() + .args(&["-d", "\n", "-s", "-f", "2"]) + .pipe_in("abc\n\n") + .succeeds() + .stdout_only("\n"); + + // Requesting both fields + new_ucmd!() + .args(&["-d", "\n", "-s", "-f", "1,2"]) + .pipe_in("abc\n\n") + .succeeds() + .stdout_only("abc\n\n"); +} + +#[test] +fn test_newline_as_delimiter_only_newlines() { + // Extracting empty fields from a string of just newlines + new_ucmd!() + .args(&["-d", "\n", "-s", "-f", "1"]) + .pipe_in("\n\n") + .succeeds() + .stdout_only("\n"); + + new_ucmd!() + .args(&["-d", "\n", "-s", "-f", "2"]) + .pipe_in("\n\n") + .succeeds() + .stdout_only("\n"); + + new_ucmd!() + .args(&["-d", "\n", "-s", "-f", "1,2"]) + .pipe_in("\n\n") + .succeeds() + .stdout_only("\n\n"); +} + +#[test] +fn test_newline_as_delimiter_last_field_no_newline() { + // The last chunk is Field 2 even without a final newline + new_ucmd!() + .args(&["-d", "\n", "-f", "2"]) + .pipe_in("abc\ndef") + .succeeds() + .stdout_only("def\n"); +} + +#[test] +fn test_newline_as_delimiter_complement() { + // Select everything except the second line + new_ucmd!() + .args(&["-d", "\n", "-f", "2", "--complement"]) + .pipe_in("line1\nline2\nline3\n") + .succeeds() + .stdout_only("line1\nline3\n"); +} + +#[test] +fn test_newline_as_delimiter_out_of_bounds() { + // GNU cut: print an empty string + terminator for missing fields + new_ucmd!() + .args(&["-d", "\n", "-f", "3"]) + .pipe_in("a\nb\n") + .succeeds() + .stdout_only("\n"); + + // GNU cut avoids trailing delimiters for out-of-bounds fields when delimiter is \n + new_ucmd!() + .args(&["-d", "\n", "-f", "1,3"]) + .pipe_in("a\nb\n") + .succeeds() + .stdout_only("a\n"); +} + +#[test] +fn test_newline_as_delimiter_no_delimiter_prints_all() { + // GNU cut: If no delimiter is found, the entire line (the whole file) + // is printed regardless of the field requested, unless -s is used. + new_ucmd!() + .args(&["-d", "\n", "-f", "2"]) + .pipe_in("a") + .succeeds() + .stdout_only("a\n"); +} + +#[test] +fn test_newline_as_delimiter_empty_input() { + new_ucmd!() + .args(&["-d", "\n", "-f", "1"]) + .pipe_in("") + .succeeds() + .no_output(); +} + +#[test] +fn test_newline_as_delimiter_s_flag_no_newline_at_all() { + new_ucmd!() + .args(&["-d", "\n", "-s", "-f", "1"]) + .pipe_in("abc") + .succeeds() + .no_output(); +} + +#[test] +fn test_newline_as_delimiter_single_field_included() { + for param in ["-s", "--only-delimited", "--only-del"] { + new_ucmd!() + .args(&["-d", "\n", param, "-f", "1"]) + .pipe_in("abc\n") + .succeeds() + .stdout_only("abc\n"); // GNU cut outputs the field + terminator + } +} + +#[test] +fn test_newline_as_delimiter_intervening_skipped_fields() { + // Selecting non-adjacent lines (Fields 1 and 3) + new_ucmd!() + .args(&["-d", "\n", "-f", "1,3"]) + .pipe_in("line1\nline2\nline3\n") + .succeeds() + .stdout_only("line1\nline3\n"); +} + +#[test] +fn test_newline_as_delimiter_multibyte_normalization() { + // Ensure multibyte records at EOF still get a normalized newline + new_ucmd!() + .args(&["-d", "\n", "-f", "2"]) + .pipe_in("\nšŸ˜¼") + .succeeds() + .stdout_only("šŸ˜¼\n"); +} + +#[test] +fn test_newline_as_delimiter_empty_first_record() { + // Select Field 2 when Field 1 is empty + new_ucmd!() + .args(&["-d", "\n", "-f", "2"]) + .pipe_in("\nb") + .succeeds() + .stdout_only("b\n"); +} + +#[test] +fn test_newline_as_delimiter_overlapping_unordered_ranges() { + // Request fields out of order and with overlapping ranges + new_ucmd!() + .args(&["-d", "\n", "-f", "2-3,1,2"]) + .pipe_in("a\nb\nc\n") + .succeeds() + .stdout_only("a\nb\nc\n"); +} + +#[test] +fn test_newline_as_delimiter_complement_last_record() { + // Test --complement on the final record + new_ucmd!() + .args(&["-d", "\n", "-f", "1", "--complement"]) + .pipe_in("a\nb") + .succeeds() + .stdout_only("b\n"); +} + #[test] fn test_multiple_delimiters() { new_ucmd!()