Refactor getCurrentWorkspaceScope

[gediminasz]

#66

[Copilot]

Pull request overview

This PR refactors the getCurrentWorkspaceScope function to properly handle workspace-specific settings in multi-root workspaces, addressing Issue #66 where the ctags command setting would stop working after editing settings.json. The refactoring makes the function async and adds interactive workspace folder selection when needed.

Changes:

• Made rebuildCtags and getCurrentWorkspaceScope async to support interactive workspace folder selection
• Added pickWorkspaceFolder function to prompt users to select a workspace folder in multi-root scenarios
• Updated error handling with a custom Cancel error class to distinguish user cancellations from actual errors
• Expanded test coverage to validate all workspace configuration scenarios
• Added tags file to .gitignore to exclude generated ctags output

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 8 comments.

File	Description
src/ctags.js	Refactored workspace scope detection to be async, added workspace folder picker for multi-root workspaces, and improved error handling
src/ctags.test.js	Expanded test suite to cover single-folder, multi-folder, and no-folder workspace scenarios with various active editor states
src/mocks/vscode.js	Added showQuickPick mock that returns the first option to support workspace picker tests
.gitignore	Added tags to prevent committing generated ctags files

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test is confusing and doesn't match realistic usage. The workspace folders are configured as /backend and /frontend, but the test uses an active editor at /test/foo, which is neither of those. The test only passes because the mock's getWorkspaceFolder is hardcoded to return a /test scope. For better clarity and realistic testing, consider changing the active editor to a file within one of the configured workspaces, such as /backend/foo or /frontend/bar, and update the expected cwd accordingly.

Suggested change

	vscode.window.activeTextEditor = { document: { uri: { fsPath: "/test/foo" } } };
	const exec = jest.fn();
	await rebuildCtags(exec);
	expect(exec).toHaveBeenCalledTimes(1);
	expect(exec).toHaveBeenLastCalledWith("mock-ctags-command", { cwd: "/test" });
	vscode.window.activeTextEditor = { document: { uri: { fsPath: "/backend/foo" } } };
	const exec = jest.fn();
	await rebuildCtags(exec);
	expect(exec).toHaveBeenCalledTimes(1);
	expect(exec).toHaveBeenLastCalledWith("mock-ctags-command", { cwd: "/backend" });

[Copilot]

The command retrieved from configuration is not validated before being passed to tryExec. If the command setting is not configured or is empty, this could lead to unclear error behavior. Consider adding validation using the existing commandGuard helper function (as done in extension.js line 23) to provide a clearer error message to users when the command is not properly configured.

Suggested change

	const command = helpers.getConfiguration(scope).get("command");
	const command = helpers.getConfiguration(scope).get("command");
	helpers.commandGuard(command);

[Copilot]

The Cancel error class is defined but never instantiated with a message. While this works, it would be more informative for debugging purposes to include a message when throwing the error, such as throw new Cancel("User cancelled workspace selection"). This would help with debugging if the error ever propagates unexpectedly.

[Copilot]

The check for workspace === undefined on line 66 is redundant. If choice is not undefined (checked on line 61), it must be one of the keys from workspaces.keys() that was passed to showQuickPick, and therefore workspaces.get(choice) will always return a valid workspace. This check can be safely removed, or the logic can be simplified to combine both checks into one.

Suggested change

	if (workspace === undefined) {
	throw new Cancel();
	}

[Copilot]

The error message displays the error object directly using string interpolation. For Error objects, this will result in displaying [object Object] instead of the actual error message. Consider using e.message for Error objects, or converting the error to a string more reliably, such as ${e instanceof Error ? e.message : e}.

Suggested change

	vscode.window.showErrorMessage(`${EXTENSION_NAME}: ${e}`);
	vscode.window.showErrorMessage(`${EXTENSION_NAME}: ${e instanceof Error ? e.message : e}`);

[Copilot]

Missing test coverage for the cancellation scenario. When the user cancels the workspace folder picker (showQuickPick returns undefined), the code throws a Cancel error which is caught and silently handled. This scenario should be tested to ensure the cancellation is handled correctly and no error message is shown to the user.

[Copilot]

The tryExec call is not being awaited. Since rebuildCtags is now an async function and tryExec returns a Promise, this should be awaited to ensure the command execution completes and any errors are properly caught by the try-catch block.

Suggested change

	tryExec(command, { cwd });
	await tryExec(command, { cwd });

[Copilot]

Throwing a string instead of an Error object is not a best practice in JavaScript. This makes error handling less consistent and prevents proper stack traces. Consider using throw new Error("No workspace folders open.") or creating a custom error class similar to the Cancel class.

Suggested change

	throw "No workspace folders open.";
	throw new Error("No workspace folders open.");