Skip to content

Do not break completion for unsupported FS providers - #5536

Merged
Patrick-Erichsen merged 1 commit into
continuedev:mainfrom
fbricon:enopro
May 7, 2025
Merged

Do not break completion for unsupported FS providers#5536
Patrick-Erichsen merged 1 commit into
continuedev:mainfrom
fbricon:enopro

Conversation

@fbricon

@fbricon fbricon commented May 6, 2025

Copy link
Copy Markdown
Contributor

Description

vscode-java uses a custom URI scheme, jdt, to represent binary class files. However it only provides a TextDocumentProvider so jdt:// URIS can be opened as documents. Because vscode-java doesn't (yet) provide a FileSystem provider, vscode.workspace.fs.readFile calls throw an ugly exception:

ENOPRO: No file system provider found for resource 'jdt://contents/java.base/java.lang/String.class?%3Ddemogorgon%2F%5C%2FUsers%5C%2Ffbricon%5C%2F.sdkman%5C%2Fcandidates%5C%2Fjava%5C%2F21.0.2-tem%5C%2Flib%5C%2Fjrt-fs.jar%60java.base%3D%2Fjavadoc_location%3D%2Fhttps%3A%5C%2F%5C%2Fdocs.oracle.com%5C%2Fen%5C%2Fjava%5C%2Fjavase%5C%2F21%5C%2Fdocs%5C%2Fapi%5C%2F%3D%2F%3D%2Fmaven.pomderived%3D%2Ftrue%3D%2F%3Cjava.lang%28String.class'

Continue ends up handling these URIs most likely after performing some LSP calls that reach the Java language server.

The fix should be 2-fold:

Basically, the idea is to centralize all vscode.workspace.fs.readFile calls in one place, and intercept/ignore such ENOPRO errors, then ignore known problematic schemes in subsequent calls.
Caveat: Similar calls to vscode.workspace.fs.stat and vscode.workspace.fs.readDirectory might be affected as well but I haven't seen it in the wild, so I didn't do anything there. [done]

Fixes #4903,
Fixes #3795,
Fixes #5226

Checklist

  • I've read the contributing guide
  • [] The relevant docs, if any, have been updated or created
  • [] The relevant tests, if any, have been updated or created

Testing instructions

Open a Java project and try to complete in a java file referencing external classes.

@fbricon
fbricon requested a review from a team as a code owner May 6, 2025 10:35
@fbricon
fbricon requested review from Patrick-Erichsen and removed request for a team May 6, 2025 10:35
@netlify

netlify Bot commented May 6, 2025

Copy link
Copy Markdown

Deploy Preview for continuedev canceled.

Name Link
🔨 Latest commit 4d1a33f
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/681b3b9484d02e00082dc637

@Patrick-Erichsen Patrick-Erichsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wow, this is a great find! I saw #5226 but didn't have a good hunch as to what the issue was since I wasn't aware of TextDocumentProvider vs FileSystemProvider.

Great implementation as well! The dynamic set is a smart solution.

Caveat: Similar calls to vscode.workspace.fs.stat and vscode.workspace.fs.readDirectory might be affected as well but I haven't seen it in the wild, so I didn't do anything there.

This feels quite likely. What are your thoughts on the following:

  • Add similar wrapper methods in extensions/vscode/src/util/ideUtils.ts
  • Don't implement, just wrap, but add a code comment indicating that we likely need to do something similar to ideUtils.readFile

There aren't many instances of either stat or readDirectory so this seems worth the effort.

Happy to merge as-is however, and I can do that myself in a follow up PR.

@fbricon

fbricon commented May 6, 2025

Copy link
Copy Markdown
Contributor Author

Ok, I'll add the other wrappers

@fbricon

fbricon commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

@Patrick-Erichsen added stat and readDirectory wrapper functions

Signed-off-by: Fred Bricon <fbricon@gmail.com>
@Patrick-Erichsen
Patrick-Erichsen merged commit 0261d47 into continuedev:main May 7, 2025
@Patrick-Erichsen

Copy link
Copy Markdown
Contributor

Excellent, appreciate the extra work adding stat and readDirectory. Thanks for the great PR!

@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 8, 2025
@fbricon

fbricon commented May 8, 2025

Copy link
Copy Markdown
Contributor Author

@Patrick-Erichsen thanks for merging. I initially messed up the syntax for automatically closing multiple issues, can you please manually close #3795 and #5226

@KeroZhai

KeroZhai commented May 9, 2025

Copy link
Copy Markdown

@Patrick-Erichsen thanks for merging. I initially messed up the syntax for automatically closing multiple issues, can you please manually close #3795 and #5226

Thanks for your great work! I think I can close #5226 myself once released (and checked).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

3 participants