diff --git a/src/context_diff.rs b/src/context_diff.rs index 86f1285..123dd63 100644 --- a/src/context_diff.rs +++ b/src/context_diff.rs @@ -373,19 +373,19 @@ pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Vec { match line { DiffLine::Context(e) => { write!(output, " ").expect("write to Vec is infallible"); - do_write_line(&mut output, &e, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &e, params.expand_tabs, params.tabsize, b" ") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } DiffLine::Change(e) => { write!(output, "! ").expect("write to Vec is infallible"); - do_write_line(&mut output, &e, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &e, params.expand_tabs, params.tabsize, b"! ") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } DiffLine::Add(e) => { write!(output, "- ").expect("write to Vec is infallible"); - do_write_line(&mut output, &e, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &e, params.expand_tabs, params.tabsize, b"- ") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } @@ -403,19 +403,19 @@ pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Vec { match line { DiffLine::Context(e) => { write!(output, " ").expect("write to Vec is infallible"); - do_write_line(&mut output, &e, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &e, params.expand_tabs, params.tabsize, b" ") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } DiffLine::Change(e) => { write!(output, "! ").expect("write to Vec is infallible"); - do_write_line(&mut output, &e, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &e, params.expand_tabs, params.tabsize, b"! ") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } DiffLine::Add(e) => { write!(output, "+ ").expect("write to Vec is infallible"); - do_write_line(&mut output, &e, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &e, params.expand_tabs, params.tabsize, b"+ ") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } diff --git a/src/ed_diff.rs b/src/ed_diff.rs index 2f75adf..6ec7356 100644 --- a/src/ed_diff.rs +++ b/src/ed_diff.rs @@ -158,7 +158,8 @@ pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Result, writeln!(&mut output, "..\n.\ns/.//").unwrap(); in_text = false; } else { - do_write_line(&mut output, actual, params.expand_tabs, params.tabsize).unwrap(); + do_write_line(&mut output, actual, params.expand_tabs, params.tabsize, b"") + .unwrap(); writeln!(&mut output).unwrap(); } } diff --git a/src/normal_diff.rs b/src/normal_diff.rs index c930b95..1ba7d03 100644 --- a/src/normal_diff.rs +++ b/src/normal_diff.rs @@ -203,7 +203,14 @@ pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Vec { } for expected in &result.expected { write!(&mut output, "< ").unwrap(); - do_write_line(&mut output, expected, params.expand_tabs, params.tabsize).unwrap(); + do_write_line( + &mut output, + expected, + params.expand_tabs, + params.tabsize, + b"< ", + ) + .unwrap(); writeln!(&mut output).unwrap(); } if result.expected_missing_nl { @@ -214,7 +221,14 @@ pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Vec { } for actual in &result.actual { write!(&mut output, "> ").unwrap(); - do_write_line(&mut output, actual, params.expand_tabs, params.tabsize).unwrap(); + do_write_line( + &mut output, + actual, + params.expand_tabs, + params.tabsize, + b"> ", + ) + .unwrap(); writeln!(&mut output).unwrap(); } if result.actual_missing_nl { diff --git a/src/unified_diff.rs b/src/unified_diff.rs index 14f4384..d6b0a4d 100644 --- a/src/unified_diff.rs +++ b/src/unified_diff.rs @@ -387,19 +387,19 @@ pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Vec { match line { DiffLine::Expected(e) => { write!(output, "-").expect("write to Vec is infallible"); - do_write_line(&mut output, &e, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &e, params.expand_tabs, params.tabsize, b"") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } DiffLine::Context(c) => { write!(output, " ").expect("write to Vec is infallible"); - do_write_line(&mut output, &c, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &c, params.expand_tabs, params.tabsize, b"") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } DiffLine::Actual(r) => { write!(output, "+",).expect("write to Vec is infallible"); - do_write_line(&mut output, &r, params.expand_tabs, params.tabsize) + do_write_line(&mut output, &r, params.expand_tabs, params.tabsize, b"") .expect("write to Vec is infallible"); writeln!(output).unwrap(); } diff --git a/src/utils.rs b/src/utils.rs index 29ab93d..1eb952f 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -16,42 +16,87 @@ use unicode_width::UnicodeWidthStr; /// Correctly handle multi-bytes characters. /// This assumes that line does not contain any line breaks (if it does, the result is undefined). #[must_use] -pub fn do_expand_tabs(line: &[u8], tabsize: usize) -> Vec { - let tab = b'\t'; - let ntabs = line.iter().filter(|c| **c == tab).count(); - if ntabs == 0 { +/// +/// The column moves the way it would on a terminal: a carriage return goes +/// back to the start of the line, and `marker` (the `< ` or `! ` in front of +/// the line) is written again after it when more text follows. A backspace +/// goes one column back, or is dropped at the start of the line, and other +/// control characters take no room. +pub fn do_expand_tabs(line: &[u8], tabsize: usize, marker: &[u8]) -> Vec { + if !line + .iter() + .any(|byte| matches!(byte, b'\t' | b'\r' | b'\x08')) + { return line.to_vec(); } - let mut result = Vec::with_capacity(line.len() + ntabs * (tabsize - 1)); - let mut offset = 0; - - let mut iter = line.split(|c| *c == tab).peekable(); - while let Some(chunk) = iter.next() { - match String::from_utf8(chunk.to_vec()) { - Ok(s) => offset += UnicodeWidthStr::width(s.as_str()), - Err(_) => offset += chunk.len(), - } - result.extend_from_slice(chunk); - if iter.peek().is_some() { - result.resize(result.len() + tabsize - offset % tabsize, b' '); - offset = 0; + let mut result = Vec::with_capacity(line.len() + tabsize); + let mut column = 0; + let mut rest = line; + while !rest.is_empty() { + let text_len = rest + .iter() + .position(u8::is_ascii_control) + .unwrap_or(rest.len()); + let (text, after) = rest.split_at(text_len); + column += text_width(text); + result.extend_from_slice(text); + let Some((&control, after)) = after.split_first() else { + break; + }; + match control { + b'\t' => { + let spaces = tabsize - column % tabsize; + result.resize(result.len() + spaces, b' '); + column += spaces; + } + b'\r' => { + column = 0; + result.push(control); + if !after.is_empty() { + result.extend_from_slice(marker); + } + } + b'\x08' if column == 0 => {} + b'\x08' => { + column -= 1; + result.push(control); + } + _ => result.push(control), } + rest = after; } result } +/// Number of columns `text` takes, where bytes that are not valid UTF-8 take +/// one column each and control characters none. +fn text_width(text: &[u8]) -> usize { + text.utf8_chunks() + .map(|chunk| { + let valid: usize = chunk + .valid() + .split(char::is_control) + .map(UnicodeWidthStr::width) + .sum(); + valid + chunk.invalid().len() + }) + .sum() +} + /// Write a single line to an output stream, expanding tabs to space if necessary. /// This assumes that line does not contain any line breaks /// (if it does and tabs are to be expanded to spaces, the result is undefined). +/// See [`do_expand_tabs`] for `marker`. pub fn do_write_line( output: &mut Vec, line: &[u8], expand_tabs: bool, tabsize: usize, + marker: &[u8], ) -> std::io::Result<()> { if expand_tabs { - output.write_all(do_expand_tabs(line, tabsize).as_slice()) + output.write_all(do_expand_tabs(line, tabsize, marker).as_slice()) } else { output.write_all(line) } @@ -395,7 +440,7 @@ mod tests { fn assert_tab_expansion(line: &str, tabsize: usize, expected: &str) { assert_eq!( - do_expand_tabs(line.as_bytes(), tabsize), + do_expand_tabs(line.as_bytes(), tabsize, b""), expected.as_bytes() ); } @@ -426,9 +471,31 @@ mod tests { fn invalid_utf8() { // [240, 240, 152, 137] is an invalid UTF-8 sequence, so it is handled as 4 bytes assert_eq!( - do_expand_tabs(&[240, 240, 152, 137, 9, 102, 111, 111], 8), + do_expand_tabs(&[240, 240, 152, 137, 9, 102, 111, 111], 8, b""), &[240, 240, 152, 137, 32, 32, 32, 32, 102, 111, 111] ); + // Only the invalid byte counts as a column, not the whole chunk. + assert_tab_expansion_bytes(b"\xff\xc3\xa9\tx", 4, b"\xff\xc3\xa9 x"); + } + + fn assert_tab_expansion_bytes(line: &[u8], tabsize: usize, expected: &[u8]) { + assert_eq!(do_expand_tabs(line, tabsize, b"< "), expected); + } + + #[test] + fn control_chars() { + // A carriage return restarts the line behind the marker again. + assert_tab_expansion_bytes(b"ab\rc\td", 8, b"ab\r< c d"); + assert_tab_expansion_bytes(b"ab\r\r\tc", 4, b"ab\r< \r< c"); + assert_tab_expansion_bytes(b"ab\r", 8, b"ab\r"); + assert_tab_expansion_bytes(b"x\r\ty\r", 4, b"x\r< y\r"); + // A backspace moves one column back, but not before the start. + assert_tab_expansion_bytes(b"abc\x08d\te", 8, b"abc\x08d e"); + assert_tab_expansion_bytes(b"\x08\x08x\ty", 8, b"x y"); + assert_tab_expansion_bytes(b"\x08x", 8, b"x"); + // Other control characters, C1 ones included, take no room. + assert_tab_expansion_bytes(b"a\x01\x7f\x1bb\tc", 8, b"a\x01\x7f\x1bb c"); + assert_tab_expansion_bytes("a\u{80}b\tc".as_bytes(), 4, "a\u{80}b c".as_bytes()); } } @@ -493,7 +560,7 @@ mod tests { fn assert_line_written(line: &str, expand_tabs: bool, tabsize: usize, expected: &str) { let mut output: Vec = Vec::new(); - assert!(do_write_line(&mut output, line.as_bytes(), expand_tabs, tabsize).is_ok()); + assert!(do_write_line(&mut output, line.as_bytes(), expand_tabs, tabsize, b"").is_ok()); assert_eq!(output, expected.as_bytes()); } diff --git a/tests/by-util/test_diff.rs b/tests/by-util/test_diff.rs index c6d4140..7b0668b 100644 --- a/tests/by-util/test_diff.rs +++ b/tests/by-util/test_diff.rs @@ -359,6 +359,68 @@ fn color_bad_option(#[case] option: &str, #[case] error: &str) { .stderr_contains(error); } +// With -t, the column moves like on a terminal: back to the start after a +// carriage return, where the line marker is printed again, and one back for +// a backspace. Other control characters take no room. +#[test] +fn expand_tabs_after_control_characters() { + let (at, ts) = at_and_ts!(); + at.write( + "old", + "ab\rc\t1\nabc\x08d\t1\n\x08x\t1\na\x01\x1bb\t1\nend\r\n", + ); + at.write( + "new", + "ab\rc\t2\nabc\x08d\t2\n\x08x\t2\na\x01\x1bb\t2\nend\r\n", + ); + let diff = |format: &str| { + ts.ucmd() + .args(&["-t", format, "old", "new"]) + .fails_with_code(1) + }; + + diff("--normal").stdout_only( + "1,4c1,4\n\ + < ab\r< c 1\n\ + < abc\x08d 1\n\ + < x 1\n\ + < a\x01\x1bb 1\n\ + ---\n\ + > ab\r> c 2\n\ + > abc\x08d 2\n\ + > x 2\n\ + > a\x01\x1bb 2\n", + ); + // The unified format does not repeat its marker. + diff("-U0").stdout_str_check(|s| { + s.ends_with( + "@@ -1,4 +1,4 @@\n\ + -ab\rc 1\n\ + -abc\x08d 1\n\ + -x 1\n\ + -a\x01\x1bb 1\n\ + +ab\rc 2\n\ + +abc\x08d 2\n\ + +x 2\n\ + +a\x01\x1bb 2\n", + ) + }); + diff("-C0").stdout_str_check(|s| { + s.ends_with( + "*** 1,4 ****\n\ + ! ab\r! c 1\n\ + ! abc\x08d 1\n\ + ! x 1\n\ + ! a\x01\x1bb 1\n\ + --- 1,4 ----\n\ + ! ab\r! c 2\n\ + ! abc\x08d 2\n\ + ! x 2\n\ + ! a\x01\x1bb 2\n", + ) + }); +} + #[cfg(target_os = "linux")] #[test] fn write_error() {