Skip to content

Commit d567cc4

Browse files
Copilotalexr00
andcommitted
Replace removeMissingRepos with reactive worktree change detection via state.onDidChange
Instead of checking filesystem on refresh, listen to each repository's state.onDidChange event and compare worktree paths. When a worktree is removed from the git state, automatically remove its folder manager. - Add checkWorktreeChanges() to RepositoriesManager that tracks worktree paths per repo and detects removals - Subscribe to state.onDidChange in registerFolderListeners - Remove removeMissingRepos() method and its call from pr.refreshList - Update MockRepository to support worktrees and fire state changes - Replace removeMissingRepos tests with worktree detection tests Co-authored-by: alexr00 <[email protected]>
1 parent 633a7c4 commit d567cc4

4 files changed

Lines changed: 94 additions & 73 deletions

File tree

src/github/repositoriesManager.ts

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,39 @@ export class RepositoriesManager extends Disposable {
9191
folderManager.onDidChangeAnyPullRequests(e => this._onDidChangeAnyPullRequests.fire(e)),
9292
folderManager.onDidAddPullRequest(e => this._onDidAddPullRequest.fire(e)),
9393
folderManager.onDidChangeGithubRepositories(() => this._onDidAddAnyGitHubRepository.fire(folderManager)),
94+
folderManager.repository.state.onDidChange(() => this.checkWorktreeChanges(folderManager.repository)),
9495
];
9596
this._subs.set(folderManager, disposables);
9697
}
9798

99+
private _previousWorktrees: Map<string, Set<string>> = new Map();
100+
101+
private checkWorktreeChanges(repo: Repository): void {
102+
const worktrees = repo.state.worktrees;
103+
if (!worktrees) {
104+
return;
105+
}
106+
107+
const repoKey = repo.rootUri.toString();
108+
const currentPaths = new Set(worktrees.map(wt => vscode.Uri.file(wt.path).toString()));
109+
const previousPaths = this._previousWorktrees.get(repoKey);
110+
this._previousWorktrees.set(repoKey, currentPaths);
111+
112+
if (!previousPaths) {
113+
return;
114+
}
115+
116+
for (const previousPath of previousPaths) {
117+
if (!currentPaths.has(previousPath)) {
118+
const folderManager = this._folderManagers.find(m => m.repository.rootUri.toString() === previousPath);
119+
if (folderManager) {
120+
Logger.appendLine(`Removing folder manager for removed worktree ${previousPath}`, RepositoriesManager.ID);
121+
this.removeRepo(folderManager.repository);
122+
}
123+
}
124+
}
125+
}
126+
98127
insertFolderManager(folderManager: FolderRepositoryManager) {
99128
this.registerFolderListeners(folderManager);
100129

@@ -134,24 +163,6 @@ export class RepositoriesManager extends Disposable {
134163
}
135164
}
136165

137-
async removeMissingRepos(): Promise<void> {
138-
const managersToRemove: FolderRepositoryManager[] = [];
139-
for (const manager of this._folderManagers) {
140-
const uri = manager.repository.rootUri;
141-
if (uri.scheme === 'file') {
142-
try {
143-
await vscode.workspace.fs.stat(uri);
144-
} catch {
145-
managersToRemove.push(manager);
146-
}
147-
}
148-
}
149-
for (const manager of managersToRemove) {
150-
Logger.appendLine(`Removing stale repository ${manager.repository.rootUri} (path no longer exists).`, RepositoriesManager.ID);
151-
this.removeRepo(manager.repository);
152-
}
153-
}
154-
155166
getManagerForIssueModel(issueModel: IssueModel | undefined): FolderRepositoryManager | undefined {
156167
if (issueModel === undefined) {
157168
return undefined;

src/test/github/repositoriesManager.test.ts

Lines changed: 53 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as vscode from 'vscode';
7-
import * as fs from 'fs';
8-
import * as os from 'os';
9-
import * as path from 'path';
107
import { SinonSandbox, createSandbox } from 'sinon';
118
import { default as assert } from 'assert';
129

@@ -143,73 +140,78 @@ describe('RepositoriesManager', function () {
143140
});
144141
});
145142

146-
describe('removeMissingRepos', function () {
147-
let tmpDir: string;
143+
describe('worktree change detection', function () {
144+
it('removes folder manager when its worktree is removed from the main repo', function () {
145+
const mainRepo = new MockRepository();
146+
mainRepo.rootUri = vscode.Uri.file('/main-repo');
147+
mainRepo.addRemote('origin', '[email protected]:aaa/bbb');
148148

149-
beforeEach(function () {
150-
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vscode-pr-test-'));
151-
});
152-
153-
afterEach(function () {
154-
fs.rmSync(tmpDir, { recursive: true, force: true });
155-
});
156-
157-
it('removes folder managers whose root URIs no longer exist on disk', async function () {
158-
const existingDir = path.join(tmpDir, 'existing-repo');
159-
fs.mkdirSync(existingDir);
149+
const worktreeRepo = new MockRepository();
150+
worktreeRepo.rootUri = vscode.Uri.file('/main-repo/worktrees/feature');
151+
worktreeRepo.addRemote('origin', '[email protected]:aaa/bbb');
160152

161-
const removedDir = path.join(tmpDir, 'removed-worktree');
162-
fs.mkdirSync(removedDir);
163-
164-
const repo1 = new MockRepository();
165-
repo1.rootUri = vscode.Uri.file(existingDir);
166-
repo1.addRemote('origin', '[email protected]:aaa/bbb');
153+
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, mainRepo, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
154+
reposManager.insertFolderManager(new FolderRepositoryManager(1, context, worktreeRepo, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
167155

168-
const repo2 = new MockRepository();
169-
repo2.rootUri = vscode.Uri.file(removedDir);
170-
repo2.addRemote('origin', '[email protected]:aaa/bbb');
156+
assert.strictEqual(reposManager.folderManagers.length, 2);
171157

172-
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo1, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
173-
reposManager.insertFolderManager(new FolderRepositoryManager(1, context, repo2, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
158+
// Set initial worktrees on the main repo (includes the worktree)
159+
mainRepo.setWorktrees([
160+
{ name: 'main-repo', path: '/main-repo', ref: 'main', main: true, detached: false },
161+
{ name: 'feature', path: '/main-repo/worktrees/feature', ref: 'feature', main: false, detached: false },
162+
]);
174163

175164
assert.strictEqual(reposManager.folderManagers.length, 2);
176165

177-
// Remove the directory to simulate worktree deletion
178-
fs.rmSync(removedDir, { recursive: true });
179-
180-
await reposManager.removeMissingRepos();
166+
// Worktree is removed - main repo state changes with updated worktrees
167+
mainRepo.setWorktrees([
168+
{ name: 'main-repo', path: '/main-repo', ref: 'main', main: true, detached: false },
169+
]);
181170

182171
assert.strictEqual(reposManager.folderManagers.length, 1);
183-
assert.strictEqual(reposManager.folderManagers[0].repository.rootUri.toString(), repo1.rootUri.toString());
172+
assert.strictEqual(reposManager.folderManagers[0].repository.rootUri.toString(), mainRepo.rootUri.toString());
184173
});
185174

186-
it('keeps all repos when all paths exist on disk', async function () {
187-
const dir1 = path.join(tmpDir, 'repo1');
188-
fs.mkdirSync(dir1);
175+
it('does not remove folder managers when worktrees remain unchanged', function () {
176+
const mainRepo = new MockRepository();
177+
mainRepo.rootUri = vscode.Uri.file('/main-repo');
178+
mainRepo.addRemote('origin', '[email protected]:aaa/bbb');
189179

190-
const dir2 = path.join(tmpDir, 'repo2');
191-
fs.mkdirSync(dir2);
180+
const worktreeRepo = new MockRepository();
181+
worktreeRepo.rootUri = vscode.Uri.file('/main-repo/worktrees/feature');
182+
worktreeRepo.addRemote('origin', '[email protected]:aaa/bbb');
192183

193-
const repo1 = new MockRepository();
194-
repo1.rootUri = vscode.Uri.file(dir1);
195-
repo1.addRemote('origin', '[email protected]:aaa/bbb');
184+
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, mainRepo, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
185+
reposManager.insertFolderManager(new FolderRepositoryManager(1, context, worktreeRepo, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
196186

197-
const repo2 = new MockRepository();
198-
repo2.rootUri = vscode.Uri.file(dir2);
199-
repo2.addRemote('origin', '[email protected]:ccc/ddd');
200-
201-
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo1, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
202-
reposManager.insertFolderManager(new FolderRepositoryManager(1, context, repo2, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
187+
// Set initial worktrees
188+
mainRepo.setWorktrees([
189+
{ name: 'main-repo', path: '/main-repo', ref: 'main', main: true, detached: false },
190+
{ name: 'feature', path: '/main-repo/worktrees/feature', ref: 'feature', main: false, detached: false },
191+
]);
203192

204-
await reposManager.removeMissingRepos();
193+
// Fire state change again with same worktrees
194+
mainRepo.setWorktrees([
195+
{ name: 'main-repo', path: '/main-repo', ref: 'main', main: true, detached: false },
196+
{ name: 'feature', path: '/main-repo/worktrees/feature', ref: 'feature', main: false, detached: false },
197+
]);
205198

206199
assert.strictEqual(reposManager.folderManagers.length, 2);
207200
});
208201

209-
it('does nothing when there are no folder managers', async function () {
210-
assert.strictEqual(reposManager.folderManagers.length, 0);
211-
await reposManager.removeMissingRepos();
212-
assert.strictEqual(reposManager.folderManagers.length, 0);
202+
it('does nothing when worktrees property is not available', function () {
203+
const repo = new MockRepository();
204+
repo.rootUri = vscode.Uri.file('/repo');
205+
repo.addRemote('origin', '[email protected]:aaa/bbb');
206+
207+
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
208+
209+
assert.strictEqual(reposManager.folderManagers.length, 1);
210+
211+
// Fire state change without setting worktrees (stays undefined)
212+
(repo as any)._onDidChangeState.fire();
213+
214+
assert.strictEqual(reposManager.folderManagers.length, 1);
213215
});
214216
});
215217
});

src/test/mocks/mockRepository.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { Uri } from 'vscode';
6+
import { EventEmitter, Uri } from 'vscode';
77
import { RefType } from '../../api/api1';
88

99
import type {
@@ -19,6 +19,7 @@ import type {
1919
BranchQuery,
2020
FetchOptions,
2121
RefQuery,
22+
Worktree,
2223
} from '../../api/api';
2324

2425
type Mutable<T> = {
@@ -67,18 +68,21 @@ export class MockRepository implements Repository {
6768
return Promise.reject(new Error(`Unexpected log(${options})`));
6869
}
6970

71+
private _onDidChangeState = new EventEmitter<void>();
72+
7073
private _state: Mutable<RepositoryState & { refs: Ref[] }> = {
7174
HEAD: {
7275
type: RefType.Head
7376
},
7477
refs: [],
7578
remotes: [],
7679
submodules: [],
80+
worktrees: undefined,
7781
rebaseCommit: undefined,
7882
mergeChanges: [],
7983
indexChanges: [],
8084
workingTreeChanges: [],
81-
onDidChange: () => ({ dispose() { } }),
85+
onDidChange: this._onDidChangeState.event,
8286
};
8387
private _config: Map<string, string> = new Map();
8488
private _branches: Branch[] = [];
@@ -337,4 +341,9 @@ export class MockRepository implements Repository {
337341
mergeAbort(): Promise<void> {
338342
return Promise.reject(new Error(`Unexpected mergeAbort`));
339343
}
344+
345+
setWorktrees(worktrees: Worktree[]) {
346+
this._state.worktrees = worktrees;
347+
this._onDidChangeState.fire();
348+
}
340349
}

src/view/prsTreeDataProvider.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
6262
this.refreshAllQueryResults(true);
6363
}
6464
}));
65-
this._register(vscode.commands.registerCommand('pr.refreshList', async _ => {
66-
await this._reposManager.removeMissingRepos();
65+
this._register(vscode.commands.registerCommand('pr.refreshList', _ => {
6766
this.prsTreeModel.forceClearCache();
6867
this.refreshAllQueryResults(true);
6968
}));

0 commit comments

Comments
 (0)