Skip to content

Commit 0455d7d

Browse files
authored
fix(builtins): change diff default output to normal format (#1022)
## Summary - Changed diff builtin default output from unified to normal (ed-style) format, matching `/usr/bin/diff` - Added `format_normal()` implementation for ed-style output (change/add/delete commands) - `-u` flag still produces unified format ## Why Scripts that parse diff output expecting the default format got wrong results because bashkit defaulted to unified format while real bash/diff defaults to normal format. ## Tests - Updated all existing unit tests to expect normal format by default - Added `test_diff_normal_format_default` — verifies ed-style output and absence of unified markers - Added `test_diff_unified_with_flag` — verifies `-u` still produces unified format - Added spec tests: `diff_default_normal_format` and `diff_unified_with_flag` - 100% bash comparison match (1692/1692) Closes #966
1 parent c2a92c3 commit 0455d7d

2 files changed

Lines changed: 177 additions & 18 deletions

File tree

crates/bashkit/src/builtins/diff.rs

Lines changed: 161 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@ use crate::interpreter::ExecResult;
1111
/// Usage: diff [-u] [-q] FILE1 FILE2
1212
///
1313
/// Options:
14-
/// -u Output in unified format (default)
14+
/// -u Output in unified format
1515
/// -q Report only whether files differ
1616
/// --brief Same as -q
17+
///
18+
/// Default output is normal (ed-style) format, matching `/usr/bin/diff`.
1719
pub struct Diff;
1820

1921
struct DiffOptions {
@@ -23,7 +25,7 @@ struct DiffOptions {
2325

2426
fn parse_diff_args(args: &[String]) -> (DiffOptions, Vec<String>) {
2527
let mut opts = DiffOptions {
26-
unified: true,
28+
unified: false,
2729
brief: false,
2830
};
2931
let mut files = Vec::new();
@@ -106,6 +108,101 @@ enum DiffLine<'a> {
106108
Removed(&'a str),
107109
}
108110

111+
/// Format diff output in normal (ed-style) format, the default for `/usr/bin/diff`.
112+
///
113+
/// Normal format uses commands like `1c1`, `2a3`, `3d2` followed by `< ` / `> ` lines.
114+
fn format_normal(diff: &[DiffLine<'_>]) -> String {
115+
let mut output = String::new();
116+
117+
let mut old_line: usize = 1;
118+
let mut new_line: usize = 1;
119+
let mut i = 0;
120+
121+
while i < diff.len() {
122+
match diff[i] {
123+
DiffLine::Context(_) => {
124+
old_line += 1;
125+
new_line += 1;
126+
i += 1;
127+
}
128+
DiffLine::Removed(_) | DiffLine::Added(_) => {
129+
// Collect contiguous removed then added lines
130+
let old_start = old_line;
131+
let new_start = new_line;
132+
let mut removed = Vec::new();
133+
let mut added = Vec::new();
134+
135+
// Collect removed lines first
136+
while i < diff.len() && matches!(diff[i], DiffLine::Removed(_)) {
137+
if let DiffLine::Removed(line) = diff[i] {
138+
removed.push(line);
139+
}
140+
old_line += 1;
141+
i += 1;
142+
}
143+
// Then collect added lines
144+
while i < diff.len() && matches!(diff[i], DiffLine::Added(_)) {
145+
if let DiffLine::Added(line) = diff[i] {
146+
added.push(line);
147+
}
148+
new_line += 1;
149+
i += 1;
150+
}
151+
152+
// Determine command type
153+
let old_end = old_start + removed.len() - if removed.is_empty() { 0 } else { 1 };
154+
let new_end = new_start + added.len() - if added.is_empty() { 0 } else { 1 };
155+
156+
if !removed.is_empty() && !added.is_empty() {
157+
// Change
158+
let old_range = if removed.len() == 1 {
159+
format!("{}", old_start)
160+
} else {
161+
format!("{},{}", old_start, old_end)
162+
};
163+
let new_range = if added.len() == 1 {
164+
format!("{}", new_start)
165+
} else {
166+
format!("{},{}", new_start, new_end)
167+
};
168+
output.push_str(&format!("{}c{}\n", old_range, new_range));
169+
for line in &removed {
170+
output.push_str(&format!("< {}\n", line));
171+
}
172+
output.push_str("---\n");
173+
for line in &added {
174+
output.push_str(&format!("> {}\n", line));
175+
}
176+
} else if !removed.is_empty() {
177+
// Delete
178+
let old_range = if removed.len() == 1 {
179+
format!("{}", old_start)
180+
} else {
181+
format!("{},{}", old_start, old_end)
182+
};
183+
output.push_str(&format!("{}d{}\n", old_range, new_start - 1));
184+
for line in &removed {
185+
output.push_str(&format!("< {}\n", line));
186+
}
187+
} else if !added.is_empty() {
188+
// Add
189+
let new_range = if added.len() == 1 {
190+
format!("{}", new_start)
191+
} else {
192+
format!("{},{}", new_start, new_end)
193+
};
194+
output.push_str(&format!("{}a{}\n", old_start - 1, new_range));
195+
for line in &added {
196+
output.push_str(&format!("> {}\n", line));
197+
}
198+
}
199+
}
200+
}
201+
}
202+
203+
output
204+
}
205+
109206
fn format_unified(file1: &str, file2: &str, diff: &[DiffLine<'_>]) -> String {
110207
let mut output = String::new();
111208

@@ -276,7 +373,11 @@ impl Builtin for Diff {
276373

277374
let diff = compute_diff(&lines1, &lines2);
278375

279-
let output = format_unified(&files[0], &files[1], &diff);
376+
let output = if opts.unified {
377+
format_unified(&files[0], &files[1], &diff)
378+
} else {
379+
format_normal(&diff)
380+
};
280381

281382
// diff returns exit code 1 when files differ, output goes to stdout
282383
Ok(ExecResult::with_code(output, 1))
@@ -342,10 +443,10 @@ mod tests {
342443
)
343444
.await;
344445
assert_eq!(result.exit_code, 1);
345-
assert!(result.stdout.contains("--- /a.txt"));
346-
assert!(result.stdout.contains("+++ /b.txt"));
347-
assert!(result.stdout.contains("-world"));
348-
assert!(result.stdout.contains("+earth"));
446+
// Default is normal format
447+
assert!(result.stdout.contains("2c2"));
448+
assert!(result.stdout.contains("< world"));
449+
assert!(result.stdout.contains("> earth"));
349450
}
350451

351452
#[tokio::test]
@@ -357,7 +458,9 @@ mod tests {
357458
)
358459
.await;
359460
assert_eq!(result.exit_code, 1);
360-
assert!(result.stdout.contains("+c"));
461+
// Normal format: add command
462+
assert!(result.stdout.contains("2a3"));
463+
assert!(result.stdout.contains("> c"));
361464
}
362465

363466
#[tokio::test]
@@ -369,7 +472,9 @@ mod tests {
369472
)
370473
.await;
371474
assert_eq!(result.exit_code, 1);
372-
assert!(result.stdout.contains("-c"));
475+
// Normal format: delete command
476+
assert!(result.stdout.contains("3d2"));
477+
assert!(result.stdout.contains("< c"));
373478
}
374479

375480
#[tokio::test]
@@ -405,7 +510,7 @@ mod tests {
405510
)
406511
.await;
407512
assert_eq!(result.exit_code, 1);
408-
assert!(result.stdout.contains("+hello"));
513+
assert!(result.stdout.contains("> hello"));
409514
}
410515

411516
#[tokio::test]
@@ -448,16 +553,18 @@ mod tests {
448553
)
449554
.await;
450555
assert_eq!(result.exit_code, 1);
451-
assert!(result.stdout.contains("-line2"));
452-
assert!(result.stdout.contains("+modified"));
556+
// Normal format: change command
557+
assert!(result.stdout.contains("2c2"));
558+
assert!(result.stdout.contains("< line2"));
559+
assert!(result.stdout.contains("> modified"));
453560
}
454561

455562
#[tokio::test]
456563
async fn test_diff_stdin() {
457564
let result = run_diff(&["-", "/b.txt"], Some("hello\n"), &[("/b.txt", b"world\n")]).await;
458565
assert_eq!(result.exit_code, 1);
459-
assert!(result.stdout.contains("-hello"));
460-
assert!(result.stdout.contains("+world"));
566+
assert!(result.stdout.contains("< hello"));
567+
assert!(result.stdout.contains("> world"));
461568
}
462569

463570
#[tokio::test]
@@ -472,9 +579,45 @@ mod tests {
472579
)
473580
.await;
474581
assert_eq!(result.exit_code, 1);
475-
assert!(result.stdout.contains("-b"));
476-
assert!(result.stdout.contains("+B"));
477-
assert!(result.stdout.contains("-d"));
478-
assert!(result.stdout.contains("+D"));
582+
// Normal format
583+
assert!(result.stdout.contains("< b"));
584+
assert!(result.stdout.contains("> B"));
585+
assert!(result.stdout.contains("< d"));
586+
assert!(result.stdout.contains("> D"));
587+
}
588+
589+
#[tokio::test]
590+
async fn test_diff_normal_format_default() {
591+
// Default format should be normal (ed-style), not unified
592+
let result = run_diff(
593+
&["/a.txt", "/b.txt"],
594+
None,
595+
&[("/a.txt", b"a\n"), ("/b.txt", b"b\n")],
596+
)
597+
.await;
598+
assert_eq!(result.exit_code, 1);
599+
assert!(result.stdout.starts_with("1c1\n"));
600+
assert!(result.stdout.contains("< a"));
601+
assert!(result.stdout.contains("---\n"));
602+
assert!(result.stdout.contains("> b"));
603+
// Should NOT contain unified format markers
604+
assert!(!result.stdout.contains("--- "));
605+
assert!(!result.stdout.contains("+++"));
606+
assert!(!result.stdout.contains("@@"));
607+
}
608+
609+
#[tokio::test]
610+
async fn test_diff_unified_with_flag() {
611+
// -u flag should produce unified format
612+
let result = run_diff(
613+
&["-u", "/a.txt", "/b.txt"],
614+
None,
615+
&[("/a.txt", b"a\n"), ("/b.txt", b"b\n")],
616+
)
617+
.await;
618+
assert_eq!(result.exit_code, 1);
619+
assert!(result.stdout.contains("--- /a.txt"));
620+
assert!(result.stdout.contains("+++ /b.txt"));
621+
assert!(result.stdout.contains("@@"));
479622
}
480623
}

crates/bashkit/tests/spec_cases/bash/diff.test.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,19 @@ echo done
3636
### expect
3737
done
3838
### end
39+
40+
### diff_default_normal_format
41+
# diff default format should be normal (ed-style)
42+
echo "a" > /tmp/diff1.txt; echo "b" > /tmp/diff2.txt
43+
diff /tmp/diff1.txt /tmp/diff2.txt | head -1
44+
### expect
45+
1c1
46+
### end
47+
48+
### diff_unified_with_flag
49+
# diff -u should produce unified format (grep for unified marker)
50+
echo "a" > /tmp/diff1.txt; echo "b" > /tmp/diff2.txt
51+
diff -u /tmp/diff1.txt /tmp/diff2.txt | grep -c "^@@"
52+
### expect
53+
1
54+
### end

0 commit comments

Comments
 (0)