Skip to content

Commit 8fc4bdb

Browse files
mtsgrdmtsgrd
authored
Merge pull request #13161 from gitbutlerapp/filter-assignments-by-files
fix: scope absorb plan and lock indicators to visible assignments
2 parents fa8bd18 + 4b9ad68 commit 8fc4bdb

5 files changed

Lines changed: 200 additions & 9 deletions

File tree

‎apps/desktop/src/components/files/FileListItems.svelte‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
8585
const filePaths = $derived(controller.changes.map((change) => change.path));
8686
const fileDependenciesQuery = $derived(
87-
showLockedIndicator ? dependencyService.filesDependencies(projectId, filePaths) : null,
87+
showLockedIndicator ? dependencyService.filesDependencies(projectId, filePaths, stackId) : null,
8888
);
8989
const fileDependencies = $derived(fileDependenciesQuery?.result.data || []);
9090
</script>

‎apps/desktop/src/lib/dependencies/dependencyService.svelte.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
aggregateFileDependencies,
3+
filterDependenciesByAssignments,
34
type FileDependencies,
45
type HunkDependencies,
56
} from "$lib/hunks/dependencies";
@@ -37,16 +38,17 @@ export default class DependencyService {
3738
);
3839
}
3940

40-
filesDependencies(projectId: string, filePaths: string[]) {
41+
filesDependencies(projectId: string, filePaths: string[], stackId?: string) {
4142
return this.worktreeService.worktreeChanges.useQuery(
4243
{ projectId },
4344
{
44-
transform: ({ dependencies }) => {
45+
transform: ({ dependencies, hunkAssignments }) => {
4546
if (!dependencies) {
4647
return [];
4748
}
4849

49-
const e = toEntityAdapter(dependencies);
50+
const filtered = filterDependenciesByAssignments(dependencies, hunkAssignments, stackId);
51+
const e = toEntityAdapter(filtered);
5052
return fileDependencySelectors.selectByIds(e.fileDependencies, filePaths);
5153
},
5254
},

‎apps/desktop/src/lib/hunks/dependencies.test.ts‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import { filterDependenciesByAssignments } from "$lib/hunks/dependencies";
2+
import { describe, expect, test } from "vitest";
3+
import type { HunkDependencies } from "$lib/hunks/dependencies";
4+
import type { HunkAssignment } from "$lib/hunks/hunk";
5+
6+
function makeDeps(
7+
entries: Array<{ path: string; newStart: number; newLines: number }>,
8+
): HunkDependencies {
9+
return {
10+
diffs: entries.map(({ path, newStart, newLines }) => [
11+
path,
12+
{ oldStart: 1, oldLines: 1, newStart, newLines, diff: "" },
13+
[{ target: { type: "stack", subject: "stack-1" }, commitId: "abc" }],
14+
]),
15+
errors: [],
16+
};
17+
}
18+
19+
function makeAssignment(
20+
path: string,
21+
stackId: string | null,
22+
newStart: number,
23+
newLines: number,
24+
): HunkAssignment {
25+
return {
26+
id: null,
27+
path,
28+
pathBytes: [],
29+
stackId,
30+
hunkHeader: { oldStart: 1, oldLines: 1, newStart, newLines },
31+
lineNumsAdded: null,
32+
lineNumsRemoved: null,
33+
};
34+
}
35+
36+
describe("filterDependenciesByAssignments", () => {
37+
describe("unassigned view (stackId = undefined)", () => {
38+
test("keeps dependency overlapping an unassigned hunk", () => {
39+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]);
40+
const assignments = [makeAssignment("a.ts", null, 10, 5)];
41+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
42+
expect(result.diffs).toHaveLength(1);
43+
});
44+
45+
test("drops dependency overlapping a stack-assigned hunk", () => {
46+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]);
47+
const assignments = [makeAssignment("a.ts", "stack-1", 10, 5)];
48+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
49+
expect(result.diffs).toHaveLength(0);
50+
});
51+
52+
test("keeps only the entry overlapping the unassigned hunk when mixed", () => {
53+
const deps = makeDeps([
54+
{ path: "a.ts", newStart: 10, newLines: 5 }, // overlaps unassigned hunk
55+
{ path: "a.ts", newStart: 50, newLines: 5 }, // overlaps stack-assigned hunk
56+
]);
57+
const assignments = [
58+
makeAssignment("a.ts", null, 10, 5),
59+
makeAssignment("a.ts", "stack-1", 50, 5),
60+
];
61+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
62+
expect(result.diffs).toHaveLength(1);
63+
expect(result.diffs[0]![1].newStart).toBe(10);
64+
});
65+
});
66+
67+
describe("stack lane view (stackId = 'stack-1')", () => {
68+
test("keeps dependency overlapping a hunk assigned to this stack", () => {
69+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]);
70+
const assignments = [makeAssignment("a.ts", "stack-1", 10, 5)];
71+
const result = filterDependenciesByAssignments(deps, assignments, "stack-1");
72+
expect(result.diffs).toHaveLength(1);
73+
});
74+
75+
test("drops dependency overlapping an unassigned hunk", () => {
76+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]);
77+
const assignments = [makeAssignment("a.ts", null, 10, 5)];
78+
const result = filterDependenciesByAssignments(deps, assignments, "stack-1");
79+
expect(result.diffs).toHaveLength(0);
80+
});
81+
82+
test("drops dependency overlapping a hunk from a different stack", () => {
83+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]);
84+
const assignments = [makeAssignment("a.ts", "stack-2", 10, 5)];
85+
const result = filterDependenciesByAssignments(deps, assignments, "stack-1");
86+
expect(result.diffs).toHaveLength(0);
87+
});
88+
});
89+
90+
describe("range overlap", () => {
91+
test("overlapping ranges match", () => {
92+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]); // [10, 15)
93+
const assignments = [makeAssignment("a.ts", null, 12, 5)]; // [12, 17)
94+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
95+
expect(result.diffs).toHaveLength(1);
96+
});
97+
98+
test("adjacent but non-overlapping ranges do not match", () => {
99+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]); // [10, 15)
100+
const assignments = [makeAssignment("a.ts", null, 15, 5)]; // [15, 20)
101+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
102+
expect(result.diffs).toHaveLength(0);
103+
});
104+
105+
test("0-line dependency hunk treated as single-line point", () => {
106+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 0 }]); // treated as [10, 11)
107+
const assignments = [makeAssignment("a.ts", null, 10, 5)]; // [10, 15)
108+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
109+
expect(result.diffs).toHaveLength(1);
110+
});
111+
112+
test("0-line assignment hunk treated as single-line point", () => {
113+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]); // [10, 15)
114+
const assignments = [makeAssignment("a.ts", null, 10, 0)]; // treated as [10, 11)
115+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
116+
expect(result.diffs).toHaveLength(1);
117+
});
118+
119+
test("no match when dependency is entirely before assignment", () => {
120+
const deps = makeDeps([{ path: "a.ts", newStart: 5, newLines: 3 }]); // [5, 8)
121+
const assignments = [makeAssignment("a.ts", null, 10, 5)]; // [10, 15)
122+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
123+
expect(result.diffs).toHaveLength(0);
124+
});
125+
});
126+
127+
describe("path matching", () => {
128+
test("only matches entries with the same path", () => {
129+
const deps = makeDeps([{ path: "a.ts", newStart: 10, newLines: 5 }]);
130+
const assignments = [makeAssignment("b.ts", null, 10, 5)];
131+
const result = filterDependenciesByAssignments(deps, assignments, undefined);
132+
expect(result.diffs).toHaveLength(0);
133+
});
134+
});
135+
136+
describe("preserves errors", () => {
137+
test("errors field is passed through unchanged", () => {
138+
const deps: HunkDependencies = {
139+
diffs: [],
140+
errors: [{ errorMessage: "oops", stackId: "s", commitId: "c", path: "a.ts" }],
141+
};
142+
const result = filterDependenciesByAssignments(deps, [], undefined);
143+
expect(result.errors).toEqual(deps.errors);
144+
});
145+
});
146+
});

‎apps/desktop/src/lib/hunks/dependencies.ts‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { DiffHunk } from "$lib/hunks/hunk";
1+
import type { DiffHunk, HunkAssignment } from "$lib/hunks/hunk";
22

33
export type DependencyError = {
44
description: string;
@@ -92,6 +92,48 @@ export type FileDependencies = {
9292
*/
9393
dependencies: HunkLocks[];
9494
};
95+
/**
96+
* Check whether two line ranges overlap.
97+
* Ranges are `[start, start + lines)` (1-based start, length in lines).
98+
* A range with 0 lines (pure insertion/deletion) is treated as a point at `start`.
99+
*/
100+
function rangesOverlap(startA: number, linesA: number, startB: number, linesB: number): boolean {
101+
const endA = startA + Math.max(linesA, 1);
102+
const endB = startB + Math.max(linesB, 1);
103+
return startA < endB && startB < endA;
104+
}
105+
106+
/**
107+
* Filters dependency entries to only those whose hunk ranges overlap with
108+
* assignments visible in the current view.
109+
*
110+
* When `stackId` is provided, includes only dependencies overlapping assignments
111+
* assigned to that stack (matching the stack lane's visible hunks).
112+
* When `stackId` is undefined, includes only dependencies overlapping
113+
* unassigned assignments (matching the unassigned lane).
114+
*/
115+
export function filterDependenciesByAssignments(
116+
dependencies: HunkDependencies,
117+
assignments: HunkAssignment[],
118+
stackId: string | undefined,
119+
): HunkDependencies {
120+
const filtered = dependencies.diffs.filter(([depPath, depHunk]) => {
121+
return assignments.some(
122+
(a) =>
123+
a.path === depPath &&
124+
a.hunkHeader !== null &&
125+
a.stackId === (stackId ?? null) &&
126+
rangesOverlap(
127+
depHunk.newStart,
128+
depHunk.newLines,
129+
a.hunkHeader.newStart,
130+
a.hunkHeader.newLines,
131+
),
132+
);
133+
});
134+
return { diffs: filtered, errors: dependencies.errors };
135+
}
136+
95137
/**
96138
* Aggregates file dependencies from a collection of hunk dependencies.
97139
*

‎crates/but-api/src/legacy/absorb.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,18 +177,19 @@ pub fn absorption_plan_with_perm(
177177
let all_assignments = worktree_changes.assignments;
178178
let dependencies = worktree_changes.dependencies;
179179

180-
// Filter assignments to just this stack
180+
// Include hunks that are unassigned or assigned to the acting stack,
181+
// so that dependency locks can route unassigned hunks correctly.
181182
let stack_assignments: Vec<_> = all_assignments
182183
.iter()
183184
.filter(|a| {
184-
a.stack_id == assigned_stack_id
185-
&& changes.iter().any(|c| c.path_bytes == a.path_bytes)
185+
changes.iter().any(|c| c.path_bytes == a.path_bytes)
186+
&& (a.stack_id.is_none() || a.stack_id == assigned_stack_id)
186187
})
187188
.cloned()
188189
.collect();
189190

190191
if stack_assignments.is_empty() {
191-
anyhow::bail!("No uncommitted changes assigned to stack: {assigned_stack_id:?}");
192+
anyhow::bail!("No uncommitted changes found for the selected files");
192193
}
193194

194195
(stack_assignments, dependencies)

0 commit comments

Comments
 (0)