Skip to content

Commit 33fedd3

Browse files
author
Test
committed
fix(worktrees): pass worktree path to tree actions and move title buttons into the view
VS Code invokes view/item/context (including inline) commands with the tree element itself as args[0], not TreeItem.command.arguments. Every action in WorktreeTreeActionCommand (open/terminal/copyWip/remove/copyPath/reveal/ addToWorkspace) declared its argument as a path string, so remove threw "paths[0] argument must be of type string" via path.resolve(), and the other actions silently misbehaved on the object. resolveWorktreeArg() normalizes string/Uri/tree-item shapes before they reach any git or fs call. Also moves "Move to New Worktree" and "Remove Multiple Worktrees" out of the view/title toolbar (they had no icon, so VS Code rendered them as text and crowded out the "GIT SMART CHECKOUT: WORKTREES" header) into a labelled action-row section at the top of the tree itself.
1 parent 65e839e commit 33fedd3

7 files changed

Lines changed: 179 additions & 20 deletions

package.json

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@
233233
{
234234
"command": "git-smart-checkout.worktree.open",
235235
"title": "Open Worktree in New Window",
236+
"icon": "$(folder-opened)",
236237
"category": "GSC"
237238
},
238239
{
@@ -329,16 +330,6 @@
329330
"when": "view == git-smart-checkout.worktrees",
330331
"group": "navigation@1"
331332
},
332-
{
333-
"command": "git-smart-checkout.moveToNewWorktree",
334-
"when": "view == git-smart-checkout.worktrees",
335-
"group": "navigation@2"
336-
},
337-
{
338-
"command": "git-smart-checkout.removeMultipleWorktrees",
339-
"when": "view == git-smart-checkout.worktrees && git-smart-checkout.hasMultipleRemovableWorktrees",
340-
"group": "navigation@3"
341-
},
342333
{
343334
"command": "git-smart-checkout.prCancelCloneMenu",
344335
"when": "view == git-smart-checkout.prClone && git-smart-checkout.isCloning == true && git-smart-checkout.isConflict == true",
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import * as vscode from 'vscode';
2+
3+
export interface ResolvedWorktreeArg {
4+
worktreePath: string;
5+
repositoryPath?: string;
6+
}
7+
8+
/**
9+
* `view/item/context` (including `inline`) commands are invoked by VS Code with the tree
10+
* element itself as the first argument, not `TreeItem.command.arguments` (those only apply
11+
* to the row's default click command). Normalizes whatever shape shows up — a plain string,
12+
* a Uri, or a WorktreeTreeItem — into a path string.
13+
*/
14+
export function resolveWorktreeArg(arg: unknown, repositoryPath?: string): ResolvedWorktreeArg | undefined {
15+
if (typeof arg === 'string') {
16+
return arg ? { worktreePath: arg, repositoryPath } : undefined;
17+
}
18+
if (arg instanceof vscode.Uri) {
19+
return { worktreePath: arg.fsPath, repositoryPath };
20+
}
21+
if (isWorktreeTreeItemLike(arg)) {
22+
return { worktreePath: arg.worktree.path, repositoryPath: arg.repositoryPath ?? repositoryPath };
23+
}
24+
return undefined;
25+
}
26+
27+
function isWorktreeTreeItemLike(
28+
arg: unknown
29+
): arg is { worktree: { path: string }; repositoryPath?: string } {
30+
if (typeof arg !== 'object' || arg === null) {
31+
return false;
32+
}
33+
const candidate = arg as { worktree?: unknown; repositoryPath?: unknown };
34+
return (
35+
typeof candidate.worktree === 'object' &&
36+
candidate.worktree !== null &&
37+
typeof (candidate.worktree as { path?: unknown }).path === 'string' &&
38+
(candidate.repositoryPath === undefined || typeof candidate.repositoryPath === 'string')
39+
);
40+
}

src/commands/utils/worktreeRemoval.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ export function isSameOrChildPath(candidatePath: string, parentPath: string): bo
2727
}
2828

2929
export function normalizePathForComparison(targetPath: string): string {
30+
if (typeof targetPath !== 'string') {
31+
return '';
32+
}
3033
try {
3134
return fs.realpathSync.native(targetPath);
3235
} catch {

src/commands/worktreeTreeActionCommand.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { RemoveWorktreeCommand } from './removeWorktreeCommand';
55
import { VscodeGitProvider } from '../common/git/vscodeGitProvider';
66
import { LoggingService } from '../logging/loggingService';
77
import { addToWorkspace } from './utils/worktreeCompletionActions';
8+
import { resolveWorktreeArg } from './utils/resolveWorktreeArg';
89
import { BaseCommand } from './command';
910

1011
export type WorktreeTreeAction =
@@ -31,8 +32,12 @@ export class WorktreeTreeActionCommand extends BaseCommand {
3132
super(logService);
3233
}
3334

34-
async execute(worktreePath?: string, repositoryPath?: string): Promise<void> {
35-
if (!worktreePath) return;
35+
async execute(arg?: unknown, repositoryPathArg?: string): Promise<void> {
36+
const resolved = resolveWorktreeArg(arg, repositoryPathArg);
37+
if (!resolved) {
38+
return;
39+
}
40+
const { worktreePath, repositoryPath } = resolved;
3641

3742
switch (this.action) {
3843
case 'open':
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import * as assert from 'assert';
2+
import * as vscode from 'vscode';
3+
4+
import { resolveWorktreeArg } from '../../commands/utils/resolveWorktreeArg';
5+
6+
describe('resolveWorktreeArg', () => {
7+
it('passes through a plain string path', () => {
8+
assert.deepStrictEqual(resolveWorktreeArg('/repo/feature', '/repo'), {
9+
worktreePath: '/repo/feature',
10+
repositoryPath: '/repo',
11+
});
12+
});
13+
14+
it('extracts fsPath from a Uri', () => {
15+
const uri = vscode.Uri.file('/repo/feature');
16+
assert.deepStrictEqual(resolveWorktreeArg(uri, '/repo'), {
17+
worktreePath: '/repo/feature',
18+
repositoryPath: '/repo',
19+
});
20+
});
21+
22+
it('extracts the path from a WorktreeTreeItem-shaped object, the argument VS Code actually sends for inline/context-menu commands', () => {
23+
const fakeTreeItem = {
24+
worktree: { path: '/repo/feature' },
25+
repositoryPath: '/repo',
26+
contextValue: 'worktree linked clean',
27+
};
28+
assert.deepStrictEqual(resolveWorktreeArg(fakeTreeItem), {
29+
worktreePath: '/repo/feature',
30+
repositoryPath: '/repo',
31+
});
32+
});
33+
34+
it('falls back to a separately passed repositoryPath when the tree item has none', () => {
35+
const fakeTreeItem = { worktree: { path: '/repo/feature' } };
36+
assert.deepStrictEqual(resolveWorktreeArg(fakeTreeItem, '/repo'), {
37+
worktreePath: '/repo/feature',
38+
repositoryPath: '/repo',
39+
});
40+
});
41+
42+
it('returns undefined for unrecognized shapes', () => {
43+
assert.strictEqual(resolveWorktreeArg(undefined), undefined);
44+
assert.strictEqual(resolveWorktreeArg(null), undefined);
45+
assert.strictEqual(resolveWorktreeArg(''), undefined);
46+
assert.strictEqual(resolveWorktreeArg(42), undefined);
47+
assert.strictEqual(resolveWorktreeArg({}), undefined);
48+
assert.strictEqual(resolveWorktreeArg({ worktree: {} }), undefined);
49+
});
50+
});

src/test/unit/worktreeTreeDataProvider.test.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import * as assert from 'assert';
22
import * as os from 'os';
3-
import { WorktreeTreeDataProvider, WorktreeTreeItem } from '../../view/WorktreeTreeDataProvider';
3+
import {
4+
WorktreeActionTreeItem,
5+
WorktreeRepositoryTreeItem,
6+
WorktreeTreeDataProvider,
7+
WorktreeTreeItem,
8+
} from '../../view/WorktreeTreeDataProvider';
49
import { PRReviewWorktreeStore } from '../../services/prReviewWorktreeStore';
510
import { mockLogService } from '../e2e/helpers/mockLogService';
611

@@ -74,6 +79,35 @@ describe('WorktreeTreeItem', () => {
7479
});
7580
});
7681

82+
describe('WorktreeRepositoryTreeItem', () => {
83+
it('uses a contextValue that does not collide with the \\bworktree\\b menu matcher', () => {
84+
const item = new WorktreeRepositoryTreeItem('/repo', []);
85+
assert.strictEqual(item.contextValue, 'worktreeRepository');
86+
assert.ok(!/\bworktree\b/.test(item.contextValue));
87+
});
88+
});
89+
90+
describe('WorktreeTreeDataProvider.getChildren action rows', () => {
91+
function makeProvider() {
92+
const store = { getForRepository: async () => [] } as unknown as PRReviewWorktreeStore;
93+
return new WorktreeTreeDataProvider(mockLogService, store);
94+
}
95+
96+
it('lists a "Move to New Worktree" action row before any worktrees, with no "Remove Multiple" row when there are none to remove', async () => {
97+
const provider = makeProvider();
98+
const children = await provider.getChildren();
99+
100+
const actionRows = children.filter((child) => child instanceof WorktreeActionTreeItem);
101+
assert.strictEqual(actionRows.length, 1);
102+
assert.strictEqual(actionRows[0].label, 'Move to New Worktree…');
103+
assert.strictEqual(
104+
(actionRows[0].command as { command: string }).command,
105+
'git-smart-checkout.moveToNewWorktree'
106+
);
107+
assert.ok(!/\bworktree\b/.test(String(actionRows[0].contextValue)));
108+
});
109+
});
110+
77111
describe('WorktreeTreeDataProvider.refreshDebounced', () => {
78112
function makeProvider() {
79113
const store = { getForRepository: async () => [] } as unknown as PRReviewWorktreeStore;

src/view/WorktreeTreeDataProvider.ts

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,26 @@ export class WorktreeRepositoryTreeItem extends vscode.TreeItem {
9292
constructor(public readonly repositoryPath: string, public readonly children: WorktreeTreeItem[]) {
9393
super(path.basename(repositoryPath) || repositoryPath, vscode.TreeItemCollapsibleState.Expanded);
9494
this.description = repositoryPath;
95-
this.contextValue = 'worktree.repository';
95+
this.contextValue = 'worktreeRepository';
9696
this.iconPath = new vscode.ThemeIcon('repo');
9797
}
9898
}
9999

100-
type WorktreeNode = WorktreeTreeItem | WorktreeRepositoryTreeItem;
100+
/**
101+
* Full-width action row rendered at the top of the tree, replacing the text buttons that
102+
* used to live in the view/title toolbar (VS Code renders title-bar buttons without an
103+
* icon as text, which crowded out the view header).
104+
*/
105+
export class WorktreeActionTreeItem extends vscode.TreeItem {
106+
constructor(label: string, icon: string, command: string) {
107+
super(label, vscode.TreeItemCollapsibleState.None);
108+
this.iconPath = new vscode.ThemeIcon(icon);
109+
this.contextValue = 'worktreeAction';
110+
this.command = { command, title: label };
111+
}
112+
}
113+
114+
type WorktreeNode = WorktreeTreeItem | WorktreeRepositoryTreeItem | WorktreeActionTreeItem;
101115

102116
const DEFAULT_DEBOUNCE_MS = 2000;
103117

@@ -119,17 +133,39 @@ export class WorktreeTreeDataProvider implements vscode.TreeDataProvider<Worktre
119133
return item;
120134
}
121135

122-
async getChildren(element?: WorktreeRepositoryTreeItem): Promise<WorktreeNode[]> {
136+
async getChildren(element?: WorktreeNode): Promise<WorktreeNode[]> {
123137
if (!this.loaded) {
124138
await this.load();
125139
}
126-
if (element) {
140+
if (element instanceof WorktreeRepositoryTreeItem) {
127141
return element.children;
128142
}
129-
if (this.repositories.length > 1) {
130-
return this.repositories;
143+
if (element) {
144+
return [];
145+
}
146+
const nodes = this.repositories.length > 1 ? this.repositories : this.items;
147+
return [...this.getActionItems(), ...nodes];
148+
}
149+
150+
private getActionItems(): WorktreeActionTreeItem[] {
151+
const removableCount = this.items.filter((item) => !item.isMain).length;
152+
const actions = [
153+
new WorktreeActionTreeItem(
154+
'Move to New Worktree…',
155+
'new-folder',
156+
'git-smart-checkout.moveToNewWorktree'
157+
),
158+
];
159+
if (removableCount >= 2) {
160+
actions.push(
161+
new WorktreeActionTreeItem(
162+
'Remove Multiple Worktrees…',
163+
'trash',
164+
'git-smart-checkout.removeMultipleWorktrees'
165+
)
166+
);
131167
}
132-
return this.items;
168+
return actions;
133169
}
134170

135171
/** Reloads immediately. Used for explicit user-triggered refreshes. */

0 commit comments

Comments
 (0)