Code cleanup for hunk dependency crate

- reformat comments
- fewer derives
- tighter types
This commit is contained in:
Mattias Granlund 2024-10-25 10:18:59 +02:00
parent e94b88a53a
commit 8bdecc2f8e
6 changed files with 74 additions and 80 deletions

View File

@ -1,6 +1,8 @@
use gitbutler_stack::StackId;
#[derive(Debug, PartialEq, Clone)]
/// A struct for tracking what stack and commit a hunk belongs to as its line numbers shift with
/// new changes come in from other commits and/or stacks.
#[derive(Debug, Clone, Copy)]
pub struct HunkRange {
pub stack_id: StackId,
pub commit_id: git2::Oid,

View File

@ -3,24 +3,27 @@ use std::path::PathBuf;
use anyhow::{anyhow, Context};
use gitbutler_stack::StackId;
#[derive(Debug, Clone)]
pub struct InputStack {
pub stack_id: StackId,
pub commits: Vec<InputCommit>,
}
#[derive(Debug, Clone)]
pub struct InputCommit {
pub commit_id: git2::Oid,
pub files: Vec<InputFile>,
}
#[derive(Debug, Clone)]
pub struct InputFile {
pub path: PathBuf,
pub diffs: Vec<InputDiff>,
}
/// Please note that the From conversions and parsing of diffs exists to facilitate
/// testing, in the client code we get the line numbers from elsewhere.
#[derive(Debug, PartialEq, Clone)]
/// Please note that the From conversions and parsing of diffs exists to facilitate testing, in
/// the client code we get the line numbers from elsewhere.
#[derive(Debug, Clone)]
pub struct InputDiff {
pub old_start: u32,
pub old_lines: u32,

View File

@ -1,19 +1,15 @@
use std::{collections::HashMap, path::PathBuf};
use gitbutler_diff::{Hunk, HunkHash};
use gitbutler_diff::{GitHunk, Hunk, HunkHash};
use gitbutler_stack::StackId;
use itertools::Itertools;
use serde::Serialize;
use crate::{InputStack, WorkspaceRanges};
// Type defined in gitbutler-branch-actions and can't be imported here.
type BranchStatus = HashMap<PathBuf, Vec<gitbutler_diff::GitHunk>>;
// A hunk is locked when it depends on changes in commits that are in your
// workspace. A hunk can be locked to more than one branch if it overlaps
// with more than one committed hunk.
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Copy)]
// A hunk is locked when it depends on changes in commits that are in your workspace. A hunk can
// be locked to more than one branch if it overlaps with more than one committed hunk.
#[derive(Debug, Clone, Copy, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct HunkLock {
// TODO: Rename this stack_id.
@ -24,46 +20,43 @@ pub struct HunkLock {
pub struct HunkDependencyOptions<'a> {
// Uncommitted changes in workspace.
pub workdir: &'a BranchStatus,
pub workdir: &'a HashMap<PathBuf, Vec<GitHunk>>,
/// A nested map of committed diffs per stack, commit, and path.
/// A nested map of committed diffs per path, commit id, and stack id.
pub stacks: Vec<InputStack>,
}
/// Returns a map from hunk hash to hunk locks.
///
/// To understand if any uncommitted changes depend on (intersect) any existing
/// changes we first transform the branch specific line numbers to global ones,
/// then look for places they intersect.
/// To understand if any uncommitted changes depend on (intersect) any existing changes we first
/// transform the branch specific line numbers to the workspace, then look for places they
/// intersect.
///
/// TODO: Change terminology to talk about dependencies instead of locks.
pub fn compute_hunk_locks(
options: HunkDependencyOptions,
) -> anyhow::Result<HashMap<HunkHash, Vec<HunkLock>>> {
let HunkDependencyOptions { workdir, stacks } = options;
// Transforms stack specific line numbers to workspace line numbers.
let ranges = WorkspaceRanges::create(stacks)?;
// Transforms local line numbers to global line numbers.
let workspace_ranges = WorkspaceRanges::create(stacks)?;
let mut result = HashMap::new();
for (path, workspace_hunks) in workdir {
for hunk in workspace_hunks {
let hunk_dependencies =
workspace_ranges.intersection(path, hunk.old_start, hunk.old_lines);
let hash = Hunk::hash_diff(&hunk.diff_lines);
let locks = hunk_dependencies
Ok(workdir
.iter()
.flat_map(|(path, workspace_hunks)| {
workspace_hunks.iter().filter_map(|hunk| {
let locks = ranges
.intersection(path, hunk.old_start, hunk.old_lines)
.iter()
.map(|dependency| HunkLock {
commit_id: dependency.commit_id,
branch_id: dependency.stack_id,
})
.collect_vec();
if !locks.is_empty() {
result.insert(hash, locks);
if locks.is_empty() {
return None;
}
}
}
Ok(result)
Some((Hunk::hash_diff(&hunk.diff_lines), locks))
})
})
.collect())
}

View File

@ -5,22 +5,18 @@ use gitbutler_stack::StackId;
use crate::{HunkRange, InputDiff};
/// Adds sequential diffs from sequential commits for a specific path, and
/// shifts line numbers with additions and deletions. It is expected that
/// diffs are added one commit at a time, each time merging the already added
/// diffs with the new ones being added.
/// Adds sequential diffs from sequential commits for a specific path, and shifts line numbers
/// with additions and deletions. It is expected that diffs are added one commit at a time,
/// each time merging the already added diffs with the new ones being added.
///
/// When combining old and new diffs we process them in turn of their start
/// line, lowest first. With each addition it is possible that we conflict
/// with previous ranges (we only know start line is higher, line count can
/// be very different), but it is important to note that old ranges will
/// not conflict with old ranges, and new ranges cannot conflict with new
/// ranges.
/// When combining old and new diffs we process them in turn of their start line, lowest first.
/// With each addition it is possible that we conflict with previous ranges (we only know start
/// line is higher, line count can be very different), but it is important to note that old
/// ranges will not conflict with old ranges, and new ranges cannot conflict with new ranges.
///
/// Therefore, a) if we are processing a new diff we know it overwrites
/// anything it conflicts with, b) when processing an old diff we e.g.
/// omit it if has been overwritten.
#[derive(Debug, Default, PartialEq, Clone)]
/// Therefore, a) if we are processing a new diff we know it overwrites anything it conflicts
/// with, b) when processing an old diff we e.g. omit it if has been overwritten.
#[derive(Debug, Default)]
pub struct PathRanges {
pub hunks: Vec<HunkRange>,
commit_ids: HashSet<git2::Oid>,
@ -46,8 +42,8 @@ impl PathRanges {
let [mut i, mut j] = [0, 0];
while i < diffs.len() || j < self.hunks.len() {
// If the old start is smaller than existing new_start, or if only have
// new diffs left to process.
// If the old start is smaller than existing new_start, or if only have new diffs
// left to process.
let mut hunks = if (i < diffs.len()
&& j < self.hunks.len()
&& diffs[i].old_start < self.hunks[j].start)
@ -73,9 +69,9 @@ impl PathRanges {
Ok(())
}
pub fn intersection(&mut self, start: u32, lines: u32) -> Vec<&mut HunkRange> {
pub fn intersection(&self, start: u32, lines: u32) -> Vec<&HunkRange> {
self.hunks
.iter_mut()
.iter()
.filter(|hunk| hunk.intersects(start, lines))
.collect()
}
@ -103,7 +99,7 @@ fn add_new(
if last_hunk.start + last_hunk.lines < new_diff.old_start {
// Diffs do not overlap so we return them in order.
Ok(vec![
last_hunk.clone(),
last_hunk,
HunkRange {
commit_id,
stack_id,
@ -113,8 +109,8 @@ fn add_new(
},
])
} else if last_hunk.contains(new_diff.old_start, new_diff.old_lines) {
// Since the diff being added is from the current commit it overwrites the
// preceding one, but we need to split it in two and retain the tail.
// Since the diff being added is from the current commit it overwrites the preceding one,
// but we need to split it in two and retain the tail.
Ok(vec![
HunkRange {
commit_id: last_hunk.commit_id,
@ -164,13 +160,13 @@ fn add_new(
/// Determines how existing diff given the previous one.
fn add_existing(hunk: &HunkRange, last_hunk: Option<HunkRange>, shift: i32) -> Vec<HunkRange> {
if last_hunk.is_none() {
return vec![hunk.clone()];
return vec![*hunk];
};
let last_hunk = last_hunk.unwrap();
if hunk.start.saturating_add_signed(shift) > last_hunk.start + last_hunk.lines {
vec![
last_hunk.clone(),
last_hunk,
HunkRange {
commit_id: hunk.commit_id,
stack_id: hunk.stack_id,
@ -180,10 +176,10 @@ fn add_existing(hunk: &HunkRange, last_hunk: Option<HunkRange>, shift: i32) -> V
},
]
} else if last_hunk.contains(hunk.start.saturating_add_signed(shift), hunk.lines) {
vec![last_hunk.clone()]
vec![last_hunk]
} else {
vec![
last_hunk.clone(),
last_hunk,
HunkRange {
commit_id: hunk.commit_id,
stack_id: hunk.stack_id,

View File

@ -8,11 +8,13 @@ use itertools::Itertools;
use crate::{HunkRange, InputDiff, PathRanges};
#[derive(Debug, Default, PartialEq, Clone)]
#[derive(Debug, Default)]
pub struct StackRanges {
pub paths: HashMap<PathBuf, PathRanges>,
}
/// A struct for collecting hunk ranges by path, before they get merged into a single dimension
/// representing the workspace view.
impl StackRanges {
pub fn add(
&mut self,
@ -39,7 +41,7 @@ impl StackRanges {
.collect::<HashSet<PathBuf>>()
}
pub fn intersection(&mut self, path: &PathBuf, start: u32, lines: u32) -> Vec<&mut HunkRange> {
pub fn intersection(&mut self, path: &PathBuf, start: u32, lines: u32) -> Vec<&HunkRange> {
if let Some(deps_path) = self.paths.get_mut(path) {
return deps_path.intersection(start, lines);
}

View File

@ -7,21 +7,20 @@ use itertools::Itertools;
use crate::{HunkRange, InputCommit, InputStack, StackRanges};
#[derive(Debug, Default, PartialEq, Clone)]
#[derive(Debug)]
pub struct WorkspaceRanges {
paths: HashMap<PathBuf, Vec<HunkRange>>,
}
/// Provides blame-like functionality for looking up what commit(s) have
/// touched a specific line number range for a given path.
/// Provides blame-like functionality for looking up what commit(s) have touched a specific line
/// number range for a given path.
///
/// First it combines changes per branch sequentially by commit, allowing
/// for dependent changes where one commit introduces changes that overwrites
/// previous changes.
/// First it combines changes per branch sequentially by commit, allowing for dependent changes
/// where one commit introduces changes that overwrites previous changes.
///
/// It then combines the changes per branch into a single vector with line
/// numbers that should match the workspace commit. These per branch changes
/// are assumed and required to be independent without overlap.
/// It then combines the changes per branch into a single vector with line numbers that should
/// match the workspace commit. These per branch changes are assumed and required to be
/// independent without overlap.
impl WorkspaceRanges {
pub fn create(input_stacks: Vec<InputStack>) -> anyhow::Result<WorkspaceRanges> {
let mut stacks = vec![];
@ -51,12 +50,11 @@ impl WorkspaceRanges {
}
/// Finds commits that intersect with a given path and range combination.
pub fn intersection(&self, path: &Path, start: u32, lines: u32) -> Vec<HunkRange> {
pub fn intersection(&self, path: &Path, start: u32, lines: u32) -> Vec<&HunkRange> {
if let Some(stack_hunks) = self.paths.get(path) {
return stack_hunks
.iter()
.filter(|hunk| hunk.intersects(start, lines))
.map(|x| x.to_owned())
.collect_vec();
}
vec![]
@ -114,7 +112,7 @@ fn combine_path_ranges(path: &Path, stacks: &[StackRanges]) -> Vec<HunkRange> {
start: hunk_dep
.start
.saturating_add_signed(line_shifts[next_index]),
..hunk_dep.clone()
..*hunk_dep
});
// Advance the path specific hunk pointer.