Skip to content

Commit 48eca22

Browse files
authored
Fix: Re-parent pulled-up directory children to ensure proper checkbox refresh (#8679)
* Fix: Re-parent pulled-up directory children to ensure proper checkbox refresh When the tree has multiple top-level directories, a temporary DirectoryTreeNode is created to build the tree, then its children are pulled up to the parent. However, the children's parent pointers still referenced the phantom root, causing processCheckboxUpdates to refresh an invisible node, so checkbox state never updated visually. Re-parent all pulled-up children to the actual container node (FilesCategoryNode, PullRequestNode, or CommitNode) so ancestor walks target visible tree nodes. * test: add regression tests for parent pointer invariant after tree building Add tests to verify that pulled-up directory children are correctly re-parented to the container node, not the phantom root. This ensures processCheckboxUpdates can walk ancestors and find visible tree nodes for refresh() calls. Tests cover: - getParent() correctly returns undefined after re-parenting - ancestor walk stops at topmost visible directory, not phantom root
1 parent 35bb21b commit 48eca22

4 files changed

Lines changed: 56 additions & 0 deletions

File tree

src/test/view/treeNodes/directoryTreeNode.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,4 +206,57 @@ describe('DirectoryTreeNode', function () {
206206
assert.strictEqual(rootDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked);
207207
});
208208
});
209+
210+
describe('parent pointer after phantom-root pull-up', function () {
211+
// When a tree has multiple top-level directories, a temporary DirectoryTreeNode
212+
// with label='' (phantom root) is used to build the tree, then its children are
213+
// pulled up to the actual container. If children are not re-parented, getParent()
214+
// still returns the phantom DirectoryTreeNode and processCheckboxUpdates fires
215+
// refresh() on an invisible node, so checkbox state never updates visually.
216+
217+
it('getParent() returns undefined after child is re-parented to the container', function () {
218+
const container = createMockParent();
219+
const phantomRoot = new DirectoryTreeNode(container, '');
220+
const subDir = new DirectoryTreeNode(phantomRoot, 'src');
221+
222+
// Before re-parenting: parent is the phantom root DirectoryTreeNode
223+
assert.ok(subDir.getParent() instanceof DirectoryTreeNode, 'sanity: should point to phantom root before re-parenting');
224+
225+
// Simulate the pull-up re-parenting fix applied in filesCategoryNode / pullRequestNode / commitNode
226+
subDir.parent = container;
227+
228+
// After re-parenting: container is not a TreeNode, so getParent() returns undefined.
229+
// This means the ancestor walk in processCheckboxUpdates stops here and refresh()
230+
// is called on the visible subDir node, not the invisible phantom root.
231+
assert.strictEqual(subDir.getParent(), undefined, 'after re-parenting, getParent() should not return the phantom root');
232+
});
233+
234+
it('ancestor walk stops at the topmost visible directory after re-parenting', function () {
235+
const container = createMockParent();
236+
const phantomRoot = new DirectoryTreeNode(container, '');
237+
const topDir = new DirectoryTreeNode(phantomRoot, 'cloud');
238+
const subDir = new DirectoryTreeNode(topDir, 'helm');
239+
const file = new MockFileNode(subDir);
240+
file.checkboxState = { state: vscode.TreeItemCheckboxState.Checked };
241+
242+
(subDir._children as any[]).push(file);
243+
topDir._children.push(subDir);
244+
245+
// Re-parent: simulate fix
246+
topDir.parent = container;
247+
248+
// Walk ancestors from file, mimicking processCheckboxUpdates
249+
const ancestors: TreeNode[] = [];
250+
let current = file.getParent();
251+
while (current instanceof DirectoryTreeNode) {
252+
ancestors.push(current);
253+
current = current.getParent();
254+
}
255+
256+
// Should find subDir and topDir, but NOT the phantom root
257+
assert.strictEqual(ancestors.length, 2);
258+
assert.strictEqual(ancestors[0], subDir);
259+
assert.strictEqual(ancestors[1], topDir);
260+
});
261+
});
209262
});

src/view/treeNodes/commitNode.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ export class CommitNode extends TreeNode implements vscode.TreeItem {
111111
dirNode.finalize();
112112
if (dirNode.label === '') {
113113
// nothing on the root changed, pull children to parent
114+
dirNode._children.forEach(child => { child.parent = this; });
114115
result.push(...dirNode._children);
115116
} else {
116117
result.push(dirNode);

src/view/treeNodes/filesCategoryNode.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ export class FilesCategoryNode extends TreeNode implements vscode.TreeItem {
9393
if (dirNode.label === '') {
9494
// nothing on the root changed, pull children to parent
9595
this.directories = dirNode._children;
96+
this.directories.forEach(child => { child.parent = this; });
9697
} else {
9798
this.directories = [dirNode];
9899
}

src/view/treeNodes/pullRequestNode.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ export class PRNode extends TreeNode implements vscode.CommentingRangeProvider2
105105
dirNode.finalize();
106106
if (dirNode.label === '') {
107107
// nothing on the root changed, pull children to parent
108+
dirNode._children.forEach(child => { child.parent = this; });
108109
result.push(...dirNode._children);
109110
} else {
110111
result.push(dirNode);

0 commit comments

Comments
 (0)