Skip to content

Fix project view not updating when folders are moved or deleted #571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/common/workspace.apis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ConfigurationScope,
Disposable,
FileDeleteEvent,
FileRenameEvent,
FileSystemWatcher,
GlobPattern,
Uri,
Expand Down Expand Up @@ -63,3 +64,11 @@ export function onDidDeleteFiles(
): Disposable {
return workspace.onDidDeleteFiles(listener, thisArgs, disposables);
}

export function onDidRenameFiles(
listener: (e: FileRenameEvent) => any,
thisArgs?: any,
disposables?: Disposable[],
): Disposable {
return workspace.onDidRenameFiles(listener, thisArgs, disposables);
}
35 changes: 35 additions & 0 deletions src/features/projectManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,41 @@ export class PythonProjectManagerImpl implements PythonProjectManager {
this._onDidChangeProjects.fire(Array.from(this._projects.values()));
}

/**
* Update a project by removing the old one and adding a new one with updated properties.
* @param existingUri The URI of the project to update.
* @param newName The new name for the project (optional, defaults to old name).
* @param newUri The new URI for the project (optional, defaults to old URI).
* @param newOptions New options for the project (optional, merged with old options).
*/
async modifyProject(
existingUri: Uri,
newName?: string,
newUri?: Uri,
newOptions?: { description?: string; tooltip?: string | MarkdownString; iconPath?: IconPath },
): Promise<void> {
const project = this.get(existingUri);
if (!project) {
return;
}

// Remove the old project
this.remove(project);

// Prepare new values
const name = newName ?? project.name;
const uri = newUri ?? project.uri;
const options = {
description: newOptions?.description ?? project.description,
tooltip: newOptions?.tooltip ?? project.tooltip,
iconPath: newOptions?.iconPath ?? (project as PythonProjectsImpl).iconPath,
};

// Create and add the new project
const updatedProject = this.create(name, uri, options);
await this.add(updatedProject);
}

getProjects(uris?: Uri[]): ReadonlyArray<PythonProject> {
if (uris === undefined) {
return Array.from(this._projects.values());
Expand Down
79 changes: 78 additions & 1 deletion src/features/views/projectView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import { PythonEnvironment } from '../../api';
import { ProjectViews } from '../../common/localize';
import { createSimpleDebounce } from '../../common/utils/debounce';
import { onDidChangeConfiguration } from '../../common/workspace.apis';
import { onDidChangeConfiguration, onDidDeleteFiles, onDidRenameFiles } from '../../common/workspace.apis';
import { EnvironmentManagers, PythonProjectManager } from '../../internal.api';
import {
GlobalProjectItem,
Expand Down Expand Up @@ -68,6 +68,12 @@ export class ProjectView implements TreeDataProvider<ProjectTreeItem> {
this.debouncedUpdateProject.trigger();
}
}),
onDidRenameFiles((e) => {
this.handleFileRenames(e);
}),
onDidDeleteFiles((e) => {
this.handleFileDeletions(e);
}),
);
}

Expand Down Expand Up @@ -224,6 +230,77 @@ export class ProjectView implements TreeDataProvider<ProjectTreeItem> {
return element.parent;
}

private async handleFileRenames(e: {
readonly files: ReadonlyArray<{ readonly oldUri: Uri; readonly newUri: Uri }>;
}): Promise<void> {
const projects = this.projectManager.getProjects();

for (const { oldUri, newUri } of e.files) {
// Check if any project matches the old URI exactly or is contained within it
const affectedProjects = projects.filter((project) => {
const projectPath = project.uri.fsPath;
const oldPath = oldUri.fsPath;

// Check if the project path is the same as or is a child of the renamed path
return (
projectPath === oldPath ||
projectPath.startsWith(oldPath + '/') ||
projectPath.startsWith(oldPath + '\\')
);
});

for (const project of affectedProjects) {
const projectPath = project.uri.fsPath;
const oldPath = oldUri.fsPath;
const newPath = newUri.fsPath;

// Calculate the new project path
let newProjectPath: string;
if (projectPath === oldPath) {
// Project path is exactly the renamed path
newProjectPath = newPath;
} else {
// Project path is a child of the renamed path
const relativePath = projectPath.substring(oldPath.length);
newProjectPath = newPath + relativePath;
}

const newProjectUri = Uri.file(newProjectPath);
await this.projectManager.modifyProject(project.uri, undefined, newProjectUri);
}

if (affectedProjects.length > 0) {
// only trigger update if there are affected projects
this.debouncedUpdateProject.trigger();
}
}
}

private handleFileDeletions(e: { readonly files: ReadonlyArray<Uri> }): void {
const projects = this.projectManager.getProjects();

for (const deletedUri of e.files) {
// Check if any project matches the deleted URI exactly or is contained within it
const affectedProjects = projects.filter((project) => {
const projectPath = project.uri.fsPath;
const deletedPath = deletedUri.fsPath;

// Check if the project path is the same as or is a child of the deleted path
return (
projectPath === deletedPath ||
projectPath.startsWith(deletedPath + '/') ||
projectPath.startsWith(deletedPath + '\\')
);
});

if (affectedProjects.length > 0) {
this.projectManager.remove(affectedProjects);
// If there are affected projects, trigger an update
this.debouncedUpdateProject.trigger();
}
}
}

dispose() {
this.disposables.forEach((d) => d.dispose());
}
Expand Down
6 changes: 6 additions & 0 deletions src/internal.api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,12 @@ export interface PythonProjectManager extends Disposable {
): PythonProject;
add(pyWorkspace: PythonProject | PythonProject[]): Promise<void>;
remove(pyWorkspace: PythonProject | PythonProject[]): void;
modifyProject(
existingUri: Uri,
newName?: string,
newUri?: Uri,
newOptions?: { description?: string; tooltip?: string | MarkdownString; iconPath?: IconPath },
): Promise<void>;
getProjects(uris?: Uri[]): ReadonlyArray<PythonProject>;
get(uri: Uri): PythonProject | undefined;
onDidChangeProjects: Event<PythonProject[] | undefined>;
Expand Down
93 changes: 93 additions & 0 deletions src/test/features/projectManager.updateUri.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import assert from 'assert';
import { Uri } from 'vscode';
import { PythonProject } from '../../api';
import { PythonProjectManagerImpl } from '../../features/projectManager';

suite('Project Manager Update URI tests', () => {
let projectManager: PythonProjectManagerImpl;

setup(() => {
projectManager = new PythonProjectManagerImpl();
});

teardown(() => {
projectManager.dispose();
});

test('updateProjectUri should update existing project URI', async () => {
const oldUri = Uri.file('/path/to/old/project');
const newUri = Uri.file('/path/to/new/project');

// Create a project and manually add it to the internal map to bypass the complex add method
const project = projectManager.create('TestProject', oldUri, {
description: 'Test project',
tooltip: 'Test tooltip',
});

// Access private _projects map to manually add the project for testing
(projectManager as unknown as { _projects: Map<string, PythonProject> })._projects.set(
oldUri.toString(),
project,
);

// Verify project exists with old URI
const oldProject = projectManager.get(oldUri);
assert.ok(oldProject, 'Project should exist with old URI');
assert.equal(oldProject.uri.fsPath, oldUri.fsPath, 'Old URI should match');

// Update the project URI
await projectManager.modifyProject(oldUri, 'project', newUri);

// Verify project no longer exists with old URI
const oldProjectAfterUpdate = projectManager.get(oldUri);
assert.equal(oldProjectAfterUpdate, undefined, 'Project should not exist with old URI after update');

// Verify project exists with new URI
const newProject = projectManager.get(newUri);
assert.ok(newProject, 'Project should exist with new URI');
assert.equal(newProject.uri.fsPath, newUri.fsPath, 'New URI should match');
assert.equal(newProject.name, 'project', 'Project name should be based on new path');
assert.equal(newProject.description, 'Test project', 'Description should be preserved');
assert.equal(newProject.tooltip, 'Test tooltip', 'Tooltip should be preserved');
});

test('updateProjectUri should handle non-existent project gracefully', () => {
const oldUri = Uri.file('/path/to/nonexistent/project');
const newUri = Uri.file('/path/to/new/project');

// Try to update a project that doesn't exist
// This should not throw an error
assert.doesNotThrow(async () => {
await projectManager.modifyProject(oldUri, 'project', newUri);
}, 'Should handle non-existent project gracefully');

// Verify no project was created
const newProject = projectManager.get(newUri);
assert.equal(newProject, undefined, 'No project should be created for non-existent old project');
});

test('remove should remove multiple projects', () => {
const project1Uri = Uri.file('/path/to/project1');
const project2Uri = Uri.file('/path/to/project2');

// Create projects and manually add them to the internal map
const project1 = projectManager.create('Project1', project1Uri);
const project2 = projectManager.create('Project2', project2Uri);

// Access private _projects map to manually add projects for testing
const pmWithPrivateAccess = projectManager as unknown as { _projects: Map<string, PythonProject> };
pmWithPrivateAccess._projects.set(project1Uri.toString(), project1);
pmWithPrivateAccess._projects.set(project2Uri.toString(), project2);

// Verify both projects exist
assert.ok(projectManager.get(project1Uri), 'Project1 should exist');
assert.ok(projectManager.get(project2Uri), 'Project2 should exist');

// Remove both projects
projectManager.remove([project1, project2]);

// Verify both projects are removed
assert.equal(projectManager.get(project1Uri), undefined, 'Project1 should be removed');
assert.equal(projectManager.get(project2Uri), undefined, 'Project2 should be removed');
});
});
Loading