Skip to content

Check if /_vscode web app exists before redirecting .vscode files #1602

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

Merged

Conversation

isc-bsaviano
Copy link
Contributor

@isc-bsaviano isc-bsaviano commented Jul 1, 2025

This PR reduces the number of unnecessary for REST requests for server-side folders that cannot redirect .vscode files. To do this, I check the list of %SYS web apps to see if /_vscode is in it. I verified that VS Code behaved correct when the /_vscode web app was enabled or disabled. While working on this change, I discovered a few other issues that I've fixed:

  • Cache the list of web apps and abstract document types for all server-namespaces in the workspace.
  • Combined our onDidChangeWorkspaceFolders event handlers into a single function.
  • Always use FileSystemError when throwing errors in FileSystemProvider.

isc-rsingh
isc-rsingh previously approved these changes Jul 1, 2025
gjsjohnmurray
gjsjohnmurray previously approved these changes Jul 1, 2025
Copy link
Contributor

@gjsjohnmurray gjsjohnmurray left a comment

Choose a reason for hiding this comment

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

Slightly uneasy about having our FSP throw errors that aren't vscode.FileSystemError.* ones, but I see we already started doing this recently in another PR, so I'm going to approve this.

@isc-bsaviano
Copy link
Contributor Author

@gjsjohnmurray I can roll that change back if you think that it's more important to always return a vscode.FileSystemError. It has a constructor so I can create errors that are not one of their predefined types. That will be useful for when their few defaults don't fit.

@isc-rsingh What do you think?

@gjsjohnmurray
Copy link
Contributor

Doc for FileSystemError says (my added emphasis):

A type that filesystem providers should use to signal errors.

I'd be inclined to play safe and toe the line here.

@isc-bsaviano isc-bsaviano dismissed stale reviews from gjsjohnmurray and isc-rsingh via dd6436a July 2, 2025 10:56
@isc-bsaviano isc-bsaviano merged commit b28eab8 into intersystems-community:master Jul 2, 2025
5 checks passed
@isc-bsaviano isc-bsaviano deleted the vscode-web-app branch July 2, 2025 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants