Skip to content

Commit 5685230

Browse files
jonathantanmy2jonathantanmy2
committed
ranges: simplify calculations, fixing bugs
This is a WIP. I need to make another pass to make the identifier names consistent, add documentation, and so on, but uploading this first to check if I'm on the right track. Second most noteworthy is the fix in input_hunk_from_unified_diff. `old_start` has a special meaning when `old_lines` is 0 (same for `new_start` and `new_lines`). Most noteworthy are the bug fixes as can be seen in some tests. In crates/but-hunk-dependency/src/ranges/tests/path.rs: In removing_line_updates_range, the file goes from ? 1 1 to (in commit 1) ? a b c to (in commit 2) ? a c It can be readily seen why the existing results are wrong. In shift_is_correct_after_multiple_changes, the file ends up being 1 2 update 3 add line 1 add line 2 add line 4 4 6 update 7 add line 8 9 10 added lines at the bottom Once again, it can be readily seen why the existing results are wrong. DO NOT SUBMIT There are some insta tests that I blindly accepted the review for that I need to look into. There might be some special handling regarding deletion and recreation of files.
1 parent dbe79bd commit 5685230

File tree

9 files changed

+215
-2065
lines changed

9 files changed

+215
-2065
lines changed

‎crates/but-hunk-dependency/src/ranges/hunk.rs‎

Lines changed: 112 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
use anyhow::{Context as _, Result};
1+
use anyhow::{Context as _, Result, bail};
22
use but_core::{TreeStatusKind, ref_metadata::StackId};
33

44
use crate::utils::PaniclessSubtraction;
55

66
/// A struct for tracking what stack and commit a hunk belongs to as its line numbers shift with
77
/// new changes come in from other commits and/or stacks.
8-
#[derive(Debug, Clone, Copy, PartialEq)]
8+
#[derive(Debug, Clone, PartialEq)]
99
pub struct HunkRange {
1010
/// The kind of change that was performed on the path of the parent-diff.
1111
pub change_type: TreeStatusKind,
@@ -21,7 +21,117 @@ pub struct HunkRange {
2121
pub line_shift: i32,
2222
}
2323

24+
fn get_left_right(start: u32, lines: u32) -> Result<(i32, i32)> {
25+
let left = if lines == 0 { start } else { start - 1 };
26+
Ok((i32::try_from(left)?, i32::try_from(left + lines)?))
27+
}
28+
29+
fn set_left_right(hunk_range: &mut HunkRange, left: i32, right: i32) -> Result<()> {
30+
hunk_range.lines = u32::try_from(right - left)?;
31+
hunk_range.start = u32::try_from(if hunk_range.lines == 0 {
32+
left
33+
} else {
34+
left + 1
35+
})?;
36+
Ok(())
37+
}
38+
39+
pub(crate) struct ReceiveResult {
40+
pub left: Option<HunkRange>,
41+
pub incoming_line_shift_change: i32,
42+
pub right: Option<HunkRange>,
43+
}
44+
2445
impl HunkRange {
46+
pub(crate) fn receive(
47+
mut self,
48+
incoming_start: u32,
49+
incoming_lines: u32,
50+
) -> Result<ReceiveResult> {
51+
let (incoming_left, incoming_right) = get_left_right(incoming_start, incoming_lines)?;
52+
let (existing_left, existing_right) = get_left_right(self.start, self.lines)?;
53+
let existing_lines = self.lines;
54+
let existing_line_shift = self.line_shift;
55+
56+
// Calculate if part or all of self will remain to the left of incoming
57+
// existing L R
58+
// incoming left 1-><-2-><-3
59+
let mut left_existing = if incoming_left <= existing_left {
60+
// 1. There will be no hunk to the left of incoming.
61+
None
62+
} else if incoming_left < existing_right {
63+
// 2. There will be a trimmed hunk to the left of incoming.
64+
let mut left_existing = self.clone();
65+
set_left_right(&mut left_existing, existing_left, incoming_left)?;
66+
Some(left_existing)
67+
} else {
68+
// 3. There will be an untrimmed hunk to the left of incoming.
69+
Some(self.clone())
70+
};
71+
72+
// Calculate if part or all of self will remain to the right of incoming
73+
// existing L R
74+
// incoming right 3-><-2-><-1
75+
let mut right_existing = if incoming_right >= existing_right {
76+
// 1. There will be no hunk to the right of incoming.
77+
None
78+
} else if incoming_right > existing_left {
79+
// 2. There will be a trimmed hunk to the right of incoming.
80+
set_left_right(&mut self, incoming_right, existing_right)?;
81+
Some(self)
82+
} else {
83+
// 3. There will be an untrimmed hunk to the right of incoming.
84+
Some(self)
85+
};
86+
87+
let incoming_line_shift_change = if let (None, None) = (&left_existing, &right_existing) {
88+
// No trace of existing hunk range, so attribute all its line shift
89+
// to the incoming hunk range.
90+
existing_line_shift
91+
} else {
92+
// Deduct trimmed lines from the `line_shift` of existing hunk(s) and
93+
// add them to the `line_shift` of incoming
94+
fn get_lines(hunk_range_option: &Option<HunkRange>) -> u32 {
95+
hunk_range_option
96+
.as_ref()
97+
.map_or(0, |hunk_range| hunk_range.lines)
98+
}
99+
let existing_lines_after_trimming =
100+
get_lines(&left_existing) + get_lines(&right_existing);
101+
let Some(trimmed_lines) = existing_lines.checked_sub(existing_lines_after_trimming)
102+
else {
103+
bail!("when calculating trimmed lines")
104+
};
105+
let trimmed_lines = i32::try_from(trimmed_lines)?;
106+
if let (Some(left_existing), Some(right_existing)) =
107+
(&mut left_existing, &mut right_existing)
108+
{
109+
// Here, both `left_existing` and `right_existing` will have the original
110+
// `line_shift` from `existing_hunk_range`.
111+
// Distribute proportionally according to their `lines`.
112+
let total_lines = i32::try_from(left_existing.lines + right_existing.lines)?;
113+
let total_net_line_shift = left_existing.line_shift - trimmed_lines;
114+
left_existing.line_shift = if total_lines == 0 {
115+
// Arbitrarily split equally to avoid a division by zero.
116+
total_net_line_shift / 2
117+
} else {
118+
total_net_line_shift * i32::try_from(left_existing.lines)? / total_lines
119+
};
120+
right_existing.line_shift = total_net_line_shift - left_existing.line_shift;
121+
} else if let Some(left_existing) = &mut left_existing {
122+
left_existing.line_shift -= trimmed_lines;
123+
} else if let Some(right_existing) = &mut right_existing {
124+
right_existing.line_shift -= trimmed_lines;
125+
}
126+
trimmed_lines
127+
};
128+
Ok(ReceiveResult {
129+
left: left_existing,
130+
incoming_line_shift_change,
131+
right: right_existing,
132+
})
133+
}
134+
25135
/// See if this range intersects with the hunk identified with `start` and `lines`.
26136
pub(crate) fn intersects(&self, start: u32, lines: u32) -> Result<bool> {
27137
if self.change_type == TreeStatusKind::Deletion {
@@ -64,46 +174,4 @@ impl HunkRange {
64174

65175
Ok(self.start <= incoming_last_line && last_line >= start)
66176
}
67-
68-
pub(crate) fn contains(&self, start: u32, lines: u32) -> bool {
69-
if lines == 0 {
70-
// Special case when only adding lines.
71-
return self.start <= start && self.start + self.lines > start + 1;
72-
}
73-
start > self.start && start + lines <= self.start + self.lines
74-
}
75-
76-
pub(crate) fn covered_by(&self, start: u32, lines: u32) -> bool {
77-
if start == 0 && lines == 0 {
78-
// Special when adding lines at the top of the file.
79-
return false;
80-
}
81-
self.start >= start && self.start + self.lines <= start + lines
82-
}
83-
84-
pub(crate) fn precedes(&self, start: u32) -> Result<bool> {
85-
let last_line = (self.start + self.lines)
86-
.sub_or_err(1)
87-
.context("While calculating the last line")?;
88-
89-
Ok(last_line < start)
90-
}
91-
92-
pub(crate) fn follows(&self, start: u32, lines: u32) -> Result<bool> {
93-
if start == 0 && lines == 0 {
94-
// Special case when adding lines at the top of the file.
95-
return Ok(true);
96-
}
97-
98-
if lines == 0 {
99-
// Special case when only adding lines.
100-
return Ok(self.start > start);
101-
}
102-
103-
let incoming_last_line = (start + lines)
104-
.sub_or_err(1)
105-
.context("While calculating the last line of the incoming hunk")?;
106-
107-
Ok(self.start > incoming_last_line)
108-
}
109177
}

0 commit comments

Comments
 (0)