Skip to content
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.

Sign up for GitHub

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

Permit Modify Server-side Workspace Folder... before connection becomes active #1477

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Permit Modify Server-side Workspace Folder... before connection becomes active #1477

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8bddce5
Permit `Modify Server-side Workspace Folder...` before connection bec…
gjsjohnmurray Feb 5, 2025
a5ef7a7
Improve the fix
gjsjohnmurray Feb 5, 2025
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
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@
},
{
"command": "vscode-objectscript.modifyWsFolder",
"when": "vscode-objectscript.connectActive && workspaceFolderCount != 0"
"when": "workspaceFolderCount != 0"
},
{
"command": "vscode-objectscript.openErrorLocation",
Expand Down Expand Up @@ -606,7 +606,7 @@
},
{
"command": "vscode-objectscript.modifyWsFolder",
"when": "vscode-objectscript.connectActive && resourceScheme =~ /^isfs(-readonly)?$/ && explorerResourceIsRoot && !listMultiSelection",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the vscode-objectscript.connectActive check is fine, but why remove the resourceScheme one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because resourceScheme is undefined if you haven't opened a file yet. And in any case the file you've got open might not come from the root whose settings you're trying to alter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds really strange to me. I just opened my .code-workspace file with two isfs folders. No files have been opened besides the "welcome" webview that opens automatically. The Modify server-side workspace folder... context menu option is present. Are those not valid reproduction steps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try starting with no workspace, then add a folder from Server Manager.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now I can see it. This seems like a bug in VS Code though. The resource* context keys should be set to the Uri of the explorer tree item when you open the context menu. That works for saved workspaces upon open, but doesn't for untitled workspaces unless you open the folder that was just added.

Copy link
Contributor Author

@gjsjohnmurray gjsjohnmurray Feb 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the resource* context keys for a root folder are missing when it's a single-root workspace but are present when it's multi-root.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we report this as a bug with the VS Code team? I'm fine with removing the "connectActive" check but I don't like adding this menu option for all workspace folder roots if we don't have to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to report it. Meanwhile I've tweaked my change.

"when": "explorerResourceIsRoot && !listMultiSelection",
"group": "objectscript_modify@3"
},
{
Expand Down
14 changes: 7 additions & 7 deletions src/commands/addServerNamespaceToWorkspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,20 +397,20 @@ export async function modifyWsFolder(wsFolderUri?: vscode.Uri): Promise<void> {
if (!wsFolder) {
return;
}
if (notIsfs(wsFolder.uri)) {
vscode.window.showErrorMessage(
`Workspace folder '${wsFolder.name}' does not have scheme 'isfs' or 'isfs-readonly'.`,
"Dismiss"
);
return;
}
} else {
// Find the workspace folder for this uri
wsFolder = vscode.workspace.getWorkspaceFolder(wsFolderUri);
if (!wsFolder) {
return;
}
}
if (notIsfs(wsFolder.uri)) {
vscode.window.showErrorMessage(
`Workspace folder '${wsFolder.name}' does not have scheme 'isfs' or 'isfs-readonly'.`,
"Dismiss"
);
return;
}

// Prompt the user to modify the uri
const newUri = await modifyWsFolderUri(wsFolder.uri);
Expand Down