-
Notifications
You must be signed in to change notification settings - Fork 95
feat: progressive project tree view during import #982
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ class LanguageServerApiManager { | |||||||||||||||||||||||||||||
| private extensionApi: any; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private isServerReady: boolean = false; | ||||||||||||||||||||||||||||||
| private isServerRunning: boolean = false; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| public async ready(): Promise<boolean> { | ||||||||||||||||||||||||||||||
| if (this.isServerReady) { | ||||||||||||||||||||||||||||||
|
|
@@ -28,6 +29,29 @@ class LanguageServerApiManager { | |||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Use serverRunning() if available (API >= 0.14) for progressive loading. | ||||||||||||||||||||||||||||||
| // This resolves when the server process is alive and can handle requests, | ||||||||||||||||||||||||||||||
| // even if project imports haven't completed yet. This enables the tree view | ||||||||||||||||||||||||||||||
| // to show projects incrementally as they are imported. | ||||||||||||||||||||||||||||||
| if (!this.isServerRunning && this.extensionApi.serverRunning) { | ||||||||||||||||||||||||||||||
| await this.extensionApi.serverRunning(); | ||||||||||||||||||||||||||||||
| this.isServerRunning = true; | ||||||||||||||||||||||||||||||
| // Start background wait for full server readiness (import complete). | ||||||||||||||||||||||||||||||
| // When the server finishes importing, trigger a full refresh to replace | ||||||||||||||||||||||||||||||
| // progressive placeholder items with proper data from the server. | ||||||||||||||||||||||||||||||
| if (this.extensionApi.serverReady) { | ||||||||||||||||||||||||||||||
| this.extensionApi.serverReady().then(() => { | ||||||||||||||||||||||||||||||
| this.isServerReady = true; | ||||||||||||||||||||||||||||||
| commands.executeCommand(Commands.VIEW_PACKAGE_INTERNAL_REFRESH, /* debounce = */false); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+46
|
||||||||||||||||||||||||||||||
| this.extensionApi.serverReady().then(() => { | |
| this.isServerReady = true; | |
| commands.executeCommand(Commands.VIEW_PACKAGE_INTERNAL_REFRESH, /* debounce = */false); | |
| }); | |
| this.extensionApi.serverReady() | |
| .then(() => { | |
| this.isServerReady = true; | |
| commands.executeCommand(Commands.VIEW_PACKAGE_INTERNAL_REFRESH, /* debounce = */false); | |
| }) | |
| .catch((error: unknown) => { | |
| // Handle server readiness failure to avoid unhandled promise rejections. | |
| console.error("Java language server failed to become ready:", error); | |
| window.showErrorMessage("Java language server failed to become ready. Some features may be unavailable."); | |
| }); |
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If onDidProjectsImport fires before the first call to ready(), it sets isServerRunning = true. In that case ready() will hit the if (this.isServerRunning) { return true; } path and will never start the background serverReady() wait/refresh, so isServerReady may remain false indefinitely. Consider starting the serverReady() wait whenever the API is initialized and serverReady exists (guarded by a single shared promise), not only inside the serverRunning() branch.
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onDidClasspathUpdate now triggers an immediate refresh with debounce = false when the server is ready. Classpath updates can arrive in bursts; removing debouncing can cause repeated full tree rebuilds and extra server traffic. Consider using debounce = true here (as before) unless there’s a concrete reason an immediate refresh is required.
| commands.executeCommand(Commands.VIEW_PACKAGE_INTERNAL_REFRESH, /* debounce = */false); | |
| commands.executeCommand(Commands.VIEW_PACKAGE_INTERNAL_REFRESH, /* debounce = */true); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,11 +37,16 @@ export class DependencyDataProvider implements TreeDataProvider<ExplorerNode> { | |||||||||||||||
| * `null` means no node is pending. | ||||||||||||||||
| */ | ||||||||||||||||
| private pendingRefreshElement: ExplorerNode | undefined | null; | ||||||||||||||||
| /** Resolved when the first batch of progressive items arrives. */ | ||||||||||||||||
| private _progressiveItemsReady: Promise<void> | undefined; | ||||||||||||||||
| private _resolveProgressiveItems: (() => void) | undefined; | ||||||||||||||||
|
|
||||||||||||||||
| constructor(public readonly context: ExtensionContext) { | ||||||||||||||||
| // commands that do not send back telemetry | ||||||||||||||||
| context.subscriptions.push(commands.registerCommand(Commands.VIEW_PACKAGE_INTERNAL_REFRESH, (debounce?: boolean, element?: ExplorerNode) => | ||||||||||||||||
| this.refresh(debounce, element))); | ||||||||||||||||
| context.subscriptions.push(commands.registerCommand(Commands.VIEW_PACKAGE_INTERNAL_ADD_PROJECTS, (projectUris: string[]) => | ||||||||||||||||
| this.addProgressiveProjects(projectUris))); | ||||||||||||||||
| context.subscriptions.push(commands.registerCommand(Commands.EXPORT_JAR_REPORT, (terminalId: string, message: string) => { | ||||||||||||||||
| appendOutput(terminalId, message); | ||||||||||||||||
| })); | ||||||||||||||||
|
|
@@ -117,10 +122,35 @@ export class DependencyDataProvider implements TreeDataProvider<ExplorerNode> { | |||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public async getChildren(element?: ExplorerNode): Promise<ExplorerNode[] | undefined | null> { | ||||||||||||||||
| // Fast path: if root items are already populated by progressive loading | ||||||||||||||||
| // (addProgressiveProjects), return them directly without querying the | ||||||||||||||||
| // server, which may be blocked during long-running imports. | ||||||||||||||||
| if (!element && this._rootItems && this._rootItems.length > 0) { | ||||||||||||||||
| explorerNodeCache.saveNodes(this._rootItems); | ||||||||||||||||
| return this._rootItems; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| if (!await languageServerApiManager.ready()) { | ||||||||||||||||
| return []; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // During progressive loading (server running but not fully ready after | ||||||||||||||||
| // a clean workspace), don't enter getRootNodes() — its server queries | ||||||||||||||||
| // will block for the entire import duration. Instead, keep the TreeView | ||||||||||||||||
| // progress spinner visible by awaiting until the first progressive | ||||||||||||||||
| // notification delivers items. | ||||||||||||||||
| if (!element && !languageServerApiManager.isFullyReady()) { | ||||||||||||||||
| if (!this._rootItems || this._rootItems.length === 0) { | ||||||||||||||||
| if (!this._progressiveItemsReady) { | ||||||||||||||||
| this._progressiveItemsReady = new Promise<void>((resolve) => { | ||||||||||||||||
| this._resolveProgressiveItems = resolve; | ||||||||||||||||
| }); | ||||||||||||||||
| } | ||||||||||||||||
| await this._progressiveItemsReady; | ||||||||||||||||
|
||||||||||||||||
| await this._progressiveItemsReady; | |
| const timeoutPromise = new Promise<void>((resolve) => { | |
| // Prevent getChildren() from hanging indefinitely if no | |
| // progressive notifications are ever received. | |
| setTimeout(resolve, 30000); | |
| }); | |
| await Promise.race([this._progressiveItemsReady, timeoutPromise]); |
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During progressive loading, getChildren(element) can still call element.getChildren() for progressive ProjectNodes (because the !isFullyReady() guard only applies to the root). Since ProjectNode.loadData() queries JDTLS, expanding a progressive placeholder during import may hang on the same workspace locks this PR is trying to avoid. Consider preventing expansion until isFullyReady() (e.g., return [] for non-root nodes while not fully ready, or use a placeholder node type that reports no children until the final refresh).
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addProgressiveProjects() always appends ProjectNodes directly to _rootItems. In multi-root workspaces, the normal root structure is WorkspaceNode entries (see getRootNodes()), so this will change the tree shape during import and may even mix WorkspaceNode and ProjectNode roots if progressive updates happen after an initial root load. Consider preserving the existing root model (group projects under their WorkspaceNode, or only enable progressive root project insertion in single-folder workspaces).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ready()now returns afterserverRunning()(when available), which changes the meaning from “import complete” to “process running”. Several existing call sites (e.g., auto-refresh watcher setup, build artifact flow, upgrade scans, and tests) callready()expecting server queries likeJdtls.getProjectUris()to work; those can still block during import per the PR description. Consider keepingready()as “fully ready” and adding a separate method for “running”, or updating allready()call sites that depend on full import completion to use a new/waitable full-ready API.