Skip to content

Commit

Permalink
Print unchanged middle lines in inline diffs
Browse files Browse the repository at this point in the history
Before, unchanged lines were omitted from the inline display. Though line
numbers are correct, it provided wrong indication that these lines were
removed. With this change, all contiguous left/right-side lines are printed,
and only common context lines are omitted from the middle.

Left/right lines within a hunk aren't interleaved as before, which I think is
nice property of difft.

Closes Wilfred#704
  • Loading branch information
yuja committed Jun 14, 2024
1 parent 589b226 commit 441371b
Show file tree
Hide file tree
Showing 51 changed files with 776 additions and 35 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ nodes should be treated as atoms. This ensures comments are treated
more consistently across languages. This fixes cases in Elm where
comment differences were ignored, and may improve other languages too.

### Display

Inline display now includes unchanged lines between hunks.

## 0.58 (released 11th May 2024)

### Parsing
Expand Down
117 changes: 82 additions & 35 deletions src/display/inline.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Inline, or "unified" diff display.
use line_numbers::LineNumber;

use crate::{
constants::Side,
display::context::{calculate_after_context, calculate_before_context, opposite_positions},
Expand Down Expand Up @@ -91,64 +93,109 @@ pub(crate) fn print(
display_options.num_context_lines as usize,
);

for (lhs_line, _) in before_lines {
if let Some(lhs_line) = lhs_line {
print!(
"{} {}",
apply_line_number_color(
&format_line_num(lhs_line),
false,
Side::Left,
display_options,
),
lhs_colored_lines[lhs_line.as_usize()]
);
// Common context lines will be emitted once at first or last. Uncommon
// lines will be inserted in between. Missing lines towards the hunk
// will also be filled.
let first_rhs_line = {
let common_len = before_lines
.iter()
.take_while(|(lhs_line, rhs_line)| lhs_line.is_some() && rhs_line.is_some())
.count();
let (common_lines, uncommon_lines) = before_lines.split_at(common_len);
if let Some((_, rhs_line)) = uncommon_lines.first() {
*rhs_line // first uncommon
} else if let Some(&(_, Some(LineNumber(a)))) = common_lines.last() {
match to_rhs_iter(hunk_lines).next() {
Some(LineNumber(b)) => (a..=b).map(LineNumber).nth(1), // next of common
None => None,
}
} else {
None
}
}
};
let last_lhs_line = {
let common_len = after_lines
.iter()
.rev()
.take_while(|(lhs_line, rhs_line)| lhs_line.is_some() && rhs_line.is_some())
.count();
let (uncommon_lines, common_lines) =
after_lines.split_at(after_lines.len() - common_len);
if let Some((lhs_line, _)) = uncommon_lines.last() {
*lhs_line // last uncommon
} else if let Some(&(Some(LineNumber(b)), _)) = common_lines.first() {
match to_lhs_iter(hunk_lines).next_back() {
Some(LineNumber(a)) => (a..=b).map(LineNumber).nth_back(1), // prev of common
None => None,
}
} else {
None
}
};

let all_lhs_lines = itertools::chain!(
to_lhs_iter(&before_lines),
to_lhs_iter(hunk_lines),
last_lhs_line,
);
let all_rhs_lines = itertools::chain!(
first_rhs_line,
to_rhs_iter(hunk_lines),
to_rhs_iter(&after_lines),
);

for (lhs_line, _) in hunk_lines {
if let Some(lhs_line) = lhs_line {
if let Some((first, last)) = get_first_last(all_lhs_lines) {
let mut lhs_hunk_lines = to_lhs_iter(hunk_lines).fuse().peekable();
for lhs_line in (first.0..=last.0).map(LineNumber) {
let is_novel = lhs_hunk_lines.next_if_eq(&lhs_line).is_some();
print!(
"{} {}",
apply_line_number_color(
&format_line_num(*lhs_line),
true,
&format_line_num(lhs_line),
is_novel,
Side::Left,
display_options,
),
lhs_colored_lines[lhs_line.as_usize()]
);
}
}
for (_, rhs_line) in hunk_lines {
if let Some(rhs_line) = rhs_line {
print!(
" {}{}",
apply_line_number_color(
&format_line_num(*rhs_line),
true,
Side::Right,
display_options,
),
rhs_colored_lines[rhs_line.as_usize()]
);
}
}

for (_, rhs_line) in &after_lines {
if let Some(rhs_line) = rhs_line {
if let Some((first, last)) = get_first_last(all_rhs_lines) {
let mut rhs_hunk_lines = to_rhs_iter(hunk_lines).fuse().peekable();
for rhs_line in (first.0..=last.0).map(LineNumber) {
let is_novel = rhs_hunk_lines.next_if_eq(&rhs_line).is_some();
print!(
" {}{}",
apply_line_number_color(
&format_line_num(*rhs_line),
false,
&format_line_num(rhs_line),
is_novel,
Side::Right,
display_options,
),
rhs_colored_lines[rhs_line.as_usize()]
);
}
}

println!();
}
}

fn to_lhs_iter<T: Copy>(
items: &[(Option<T>, Option<T>)],
) -> impl DoubleEndedIterator<Item = T> + '_ {
items.iter().filter_map(|(lhs, _)| *lhs)
}

fn to_rhs_iter<T: Copy>(
items: &[(Option<T>, Option<T>)],
) -> impl DoubleEndedIterator<Item = T> + '_ {
items.iter().filter_map(|(_, rhs)| *rhs)
}

fn get_first_last<T: Copy>(mut iter: impl DoubleEndedIterator<Item = T>) -> Option<(T, T)> {
let first = iter.next()?;
let last = iter.next_back().unwrap_or(first);
Some((first, last))
}
14 changes: 14 additions & 0 deletions tests/snapshots/cli__samples_inline@Session_1.kt.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ input_file: sample_files/Session_1.kt
97   * The session's photo URL.
98   */
99  val photoUrl: String?,
100 
99  val photoUrl: String,
100 
101  /**
102  * IDs of the sessions related to this session.
103  */
Expand All @@ -18,9 +20,21 @@ input_file: sample_files/Session_1.kt
113  // available.
114  return startTime <= now && endTime >= now
115  }
116 
117  /**
118   * Returns whether the session has a video or not. A session could be live streaming or have a
119   * recorded session. Both live stream and recorded videos are stored in [Session.youTubeUrl].
120   */
121  fun hasVideo() = youTubeUrl.isNotEmpty()
116 
117  val hasPhoto inline get() = photoUrl.isNotEmpty()
118 
119  /**
120  * Returns whether the session has a video or not. A session could be live streaming or have a
121  * recorded session. Both live stream and recorded videos are stored in [Session.youTubeUrl].
122  */
123  val hasVideo inline get() = youTubeUrl.isNotEmpty()
124 
125  val hasPhotoOrVideo inline get() = hasPhoto || hasVideo
126 
127  /**
Expand Down
3 changes: 3 additions & 0 deletions tests/snapshots/cli__samples_inline@ada_1.adb.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ input_file: sample_files/ada_1.adb
sample_files/ada_2.adb --- Ada
1  with Ada.Text_IO; use Ada.Text_IO; procedure Hello is begin Put_Line ("Hello WORLD!"); end Hello;
1 with Ada.Text_IO;
2 
3 procedure Hello is
4  package TIO renames Ada.Text_IO;
5 begin
6  TIO.Put_Line ("Hello World!");
7 end Hello;
5 changes: 5 additions & 0 deletions tests/snapshots/cli__samples_inline@bad_combine_1.rs.snap
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ input_file: sample_files/bad_combine_1.rs
7  max_rhs_src_line: LineNumber,
8  ) {
9  }
10 
11  fn display_line_nums(
12  ) -> (String, String) {
13  let display_rhs_line_num: String = match rhs_line_num {
14  Some(line_num) => {
15  let s = format_line_num_padded(line_num, rhs_column_width);
5  let s = format_line_num_padded(line_num, widths.rhs_line_nums);
6  if rhs_lines_with_novel.contains(&line_num) {
Expand Down
1 change: 1 addition & 0 deletions tests/snapshots/cli__samples_inline@clojure_1.clj.snap
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ input_file: sample_files/clojure_1.clj
7  {:more (inc x)
8  :less (dec x)})
6  (-> {:more (inc x)
7  :less (dec x)}
8  (assoc :twice (+ x x))))
2 changes: 2 additions & 0 deletions tests/snapshots/cli__samples_inline@comma_1.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ input_file: sample_files/comma_1.js
13  function arrayIncludesWith(array, value, comparator) {
14  let index = -1
15  const length = array == null ? 0 : array.length
16 
17  while (++index < length) {
18  if (comparator(value, array[index])) {
13 function arrayIncludesWith(array, target, comparator) {
14  if (array == null) {
15  return false
16  }
17 
18  for (const value of array) {
19  if (comparator(target, value)) {
20  return true
Expand Down
22 changes: 22 additions & 0 deletions tests/snapshots/cli__samples_inline@comments_1.rs.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,41 @@ input_file: sample_files/comments_1.rs
2  f1();
3 
4  // Changing a single word.
5  f2();
6 
7  // A completely different sentence.
8  f3();
9 
10  if true {
11  /* A multiline comment
12   * whose indentation changes.
13   */
14  }
15 
16  // An example environment variable: FOO="a-b"
17 
18  // A single line comment.
19 
20  /** A doc comment.
21   *
22   * This line will change.
23   */
4 // Changing a single word here.
5 f2();
6 
7 // A single comment about something.
8 f3();
9 
10 /* A multiline comment
11  * whose indentation changes.
12  */
13 
14 // An example environment variable: FOO="x-y"
15 
16 // A single line comment. It has become
17 // a big block comment. Lorem ipsum dolor sit amet,
18 // consectetur adipiscing elit
19 
20 /** A doc comment.
21  *
22  * This line has changed.
Expand Down
1 change: 1 addition & 0 deletions tests/snapshots/cli__samples_inline@context_1.rs.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ input_file: sample_files/context_1.rs
3  if print_unchanged {
4  }
5  }
6 
2  match () {
3  x => {
4  let opposite_to_lhs = opposite_positions(&summary.lhs_positions);
Expand Down
2 changes: 2 additions & 0 deletions tests/snapshots/cli__samples_inline@contiguous_1.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ input_file: sample_files/contiguous_1.js
sample_files/contiguous_2.js --- JavaScript
1 // There are multiple possible diffs here, but we want to prefer
2 // showing A and B on the same line.
3 var x = [
4  "A", "B",
5  "A", "B",
6  "C", "D",
7 ];
18 changes: 18 additions & 0 deletions tests/snapshots/cli__samples_inline@css_1.css.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,37 @@ input_file: sample_files/css_1.css
2  world */
3  .foo1 {
4  margin: 0 0 20px 0;
5  }
6 
7  .bar {
8  margin: 0;
9  }
10 
11  .baz {
12  color: yellow;
13  font-family: "Before";
14  }
15 
16  .another {
17  margin-left: 0.5em;
3 .bar {
4  margin: 0;
5 }
6 
7 .foo1 {
8  margin: 0 0 20px 0;
9  color: green;
10 }
11 
12 .baz {
13  color: blue;
14  font-family: "After";
15 }
16 
17 .another {
18  margin-left: 1em;
19 }
20 
21 p {
22  color: #000;
23 }
2 changes: 2 additions & 0 deletions tests/snapshots/cli__samples_inline@dart_1.dart.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ input_file: sample_files/dart_1.dart
2  if (x) {
3  Object().a().b();
4  }
5 
6  expect(a.b(c.d()).x);
2  Object()..a()..b();
3 
4  expect(a.b.c.d()!.x);
5 }
34 changes: 34 additions & 0 deletions tests/snapshots/cli__samples_inline@devicetree_1.dts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,53 @@ input_file: sample_files/devicetree_1.dts
34  i-cache-size = <65536>;
35  d-cache-size = <32768>;
36  };
37 
38  };
39 
40  randomnode {
41  string = "\xff\0stuffstuff\t\t\t\n\n\n";
42  blob = [0a 0b 0c 0d de ea ad be ef];
43  ref = < &{/memory@0} >;
44  mixed = "abc", [1234], <0xa 0xb 0xc>;
45  old = <12345>;
46  };
47 
48  memory@0 {
49  device_type = "memory";
50  memreg: reg = <0x00000000 0x00000000 0x00000000 0x20000000>;
51  };
52 
53  chosen {
54  bootargs = "root=/dev/sda2";
55  linux,platform = <0x600>;
29  randomparentnode {
30  randomnode {
31  string = "\xff\0stuffstuff\t\t\t\n\n\n";
32  blob = [0a 0b 0c 0d de ea ad be ef];
33  ref = < &{/memory@0} >;
34  mixed = "abc",
35  [1234],
36  <0xa 0xb 0xc>;
37  new = <12345>;
38  };
39  };
40 
41  memory@0 {
42  device_type = "memory";
43  memreg: reg = <
44  0x00000000 0x00000000
45  0x00000000 0x20000000>;
46  };
47 
48  memory@100000 {
49  device_type = "memory";
50  memreg: reg = <
51  0x00100000 0x00000000
52  0x00100000 0x20000000>;
53  };
54 
55  chosen {
56  linux,platform = <0x600>;
57  bootargs = "root=/dev/sda2";
58  };
59 
Expand Down
Loading

0 comments on commit 441371b

Please sign in to comment.