-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add multi project support vscode with single server #3987
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
Changes from all commits
da8099b
53ed383
a5da352
22d1495
91e552b
d328a01
b545a22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -360,6 +360,11 @@ | |||||
| "default": true, | ||||||
| "description": "Whether to ask for permission before downloading any files from the Internet" | ||||||
| }, | ||||||
| "rust-analyzer.server.autoRestartOnNew": { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| "type": "boolean", | ||||||
| "default": false, | ||||||
| "description": "automatically restarts rust-analyzer server when a new project is discovered" | ||||||
| }, | ||||||
| "rust-analyzer.serverPath": { | ||||||
| "type": [ | ||||||
| "null", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,15 +8,76 @@ import { activateInlayHints } from './inlay_hints'; | |
| import { activateStatusDisplay } from './status_display'; | ||
| import { Ctx } from './ctx'; | ||
| import { Config, NIGHTLY_TAG } from './config'; | ||
| import { log, assert } from './util'; | ||
| import { log, assert, nearestParentWithCargoToml, createWorkspaceWithNewLocation } from './util'; | ||
| import { PersistentState } from './persistent_state'; | ||
| import { fetchRelease, download } from './net'; | ||
| import { spawnSync } from 'child_process'; | ||
| import { activateTaskProvider } from './tasks'; | ||
|
|
||
| let ctx: Ctx | undefined; | ||
| const foundProjects: Set<string> = new Set(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a general comment, global singletons should not be added without very good reason.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 100% agree, |
||
| let config: Config | undefined = undefined; | ||
|
|
||
| async function locateRustProjects(root: vscode.Uri) { | ||
| const cargoRoots = await Promise.all(vscode.workspace.textDocuments.map((doc) => nearestParentWithCargoToml(root, doc.uri))); | ||
| for (const cargoRoot of cargoRoots) { | ||
| if (cargoRoot != null) { | ||
| foundProjects.add(cargoRoot.fsPath); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export async function activate(context: vscode.ExtensionContext) { | ||
|
|
||
| config = new Config(context); | ||
|
|
||
| const workspaceFolder = vscode.workspace.workspaceFolders?.[0]; | ||
| if (workspaceFolder !== undefined) { | ||
| await locateRustProjects(workspaceFolder.uri); | ||
| } | ||
|
|
||
| vscode.workspace.onDidOpenTextDocument(async (doc) => { | ||
| const wf = vscode.workspace.getWorkspaceFolder(doc.uri); | ||
| if (wf) { | ||
| const cargoRoot = await nearestParentWithCargoToml(wf.uri, doc.uri); | ||
| if (cargoRoot != null) { | ||
| const isMissing = !foundProjects.has(cargoRoot.fsPath); | ||
|
|
||
| foundProjects.add(cargoRoot.fsPath); | ||
| if (isMissing) { | ||
| if (config?.autoRestartOnNew) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My experience in IntelliJ tells me that auto restart doesn't worth the trouble. The problem is that we should only restart in quiescent state, and only the user knows when this state is reached.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ie, setting a status "needs restart" is absolutely necessary, and a big missing piece in rust-analyzer at the moment, but it should be the user who decides to click the restart button.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added it after testing it a bit, since it got very old very quickly having to restart manually all the time, which is also why it's turned off by default. |
||
| restart(context); | ||
| } else { | ||
| vscode.window.showInformationMessage( | ||
| `Found a new project at ${cargoRoot.fsPath}.` + | ||
| " Manually run: \"Rust Analyzer: Restart server\"" + | ||
| " or set rust-analyzer.server.autoRestartOnNew=true" | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| startRA(context); | ||
|
|
||
| } | ||
|
|
||
| export async function restart(context: vscode.ExtensionContext) { | ||
| void vscode.window.showInformationMessage('Reloading rust-analyzer...'); | ||
| await deactivate(); | ||
| while (context.subscriptions.length > 0) { | ||
| try { | ||
| context.subscriptions.pop()!.dispose(); | ||
| } catch (err) { | ||
| log.error("Dispose error:", err); | ||
| } | ||
| } | ||
| await startRA(context).catch(log.error); | ||
| } | ||
|
|
||
| export async function startRA(context: vscode.ExtensionContext) { | ||
|
|
||
| // Register a "dumb" onEnter command for the case where server fails to | ||
| // start. | ||
| // | ||
|
|
@@ -31,13 +92,15 @@ export async function activate(context: vscode.ExtensionContext) { | |
| // "rust-analyzer is not available" | ||
| // ), | ||
| // ) | ||
|
|
||
| const defaultOnEnter = vscode.commands.registerCommand( | ||
| 'rust-analyzer.onEnter', | ||
| () => vscode.commands.executeCommand('default:type', { text: '\n' }), | ||
| ); | ||
| context.subscriptions.push(defaultOnEnter); | ||
|
|
||
| const config = new Config(context); | ||
| config = new Config(context); | ||
|
|
||
| const state = new PersistentState(context.globalState); | ||
| const serverPath = await bootstrap(config, state); | ||
|
|
||
|
|
@@ -52,23 +115,11 @@ export async function activate(context: vscode.ExtensionContext) { | |
| // registers its `onDidChangeDocument` handler before us. | ||
| // | ||
| // This a horribly, horribly wrong way to deal with this problem. | ||
| ctx = await Ctx.create(config, context, serverPath, workspaceFolder.uri.fsPath); | ||
|
|
||
| ctx = await Ctx.create(config, context, serverPath, workspaceFolder.uri.fsPath, Array.from(foundProjects)); | ||
| // Commands which invokes manually via command palette, shortcut, etc. | ||
|
|
||
| // Reloading is inspired by @DanTup maneuver: https://github.com/microsoft/vscode/issues/45774#issuecomment-373423895 | ||
| ctx.registerCommand('reload', _ => async () => { | ||
| void vscode.window.showInformationMessage('Reloading rust-analyzer...'); | ||
| await deactivate(); | ||
| while (context.subscriptions.length > 0) { | ||
| try { | ||
| context.subscriptions.pop()!.dispose(); | ||
| } catch (err) { | ||
| log.error("Dispose error:", err); | ||
| } | ||
| } | ||
| await activate(context).catch(log.error); | ||
| }); | ||
| ctx.registerCommand('reload', _ => restart); | ||
|
|
||
| ctx.registerCommand('analyzerStatus', commands.analyzerStatus); | ||
| ctx.registerCommand('collectGarbage', commands.collectGarbage); | ||
|
|
@@ -92,7 +143,9 @@ export async function activate(context: vscode.ExtensionContext) { | |
| ctx.registerCommand('applySourceChange', commands.applySourceChange); | ||
| ctx.registerCommand('selectAndApplySourceChange', commands.selectAndApplySourceChange); | ||
|
|
||
| ctx.pushCleanup(activateTaskProvider(workspaceFolder)); | ||
| for (const project of foundProjects) { | ||
| ctx.pushCleanup(activateTaskProvider(createWorkspaceWithNewLocation(workspaceFolder, vscode.Uri.file(project)))); | ||
| } | ||
|
|
||
| activateStatusDisplay(ctx); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| import * as lc from "vscode-languageclient"; | ||
| import * as vscode from "vscode"; | ||
| import * as path from 'path'; | ||
| import * as fs from 'fs'; | ||
| import * as util from 'util'; | ||
| import { strict as nativeAssert } from "assert"; | ||
|
|
||
| export function assert(condition: boolean, explanation: string): asserts condition { | ||
|
|
@@ -11,6 +14,7 @@ export function assert(condition: boolean, explanation: string): asserts conditi | |
| } | ||
| } | ||
|
|
||
|
|
||
| export const log = new class { | ||
| private enabled = true; | ||
|
|
||
|
|
@@ -82,3 +86,49 @@ export function isRustDocument(document: vscode.TextDocument): document is RustD | |
| export function isRustEditor(editor: vscode.TextEditor): editor is RustEditor { | ||
| return isRustDocument(editor.document); | ||
| } | ||
|
|
||
| export function createWorkspaceWithNewLocation(workspace: vscode.WorkspaceFolder, newLoc: vscode.Uri) { | ||
| return { | ||
| ...workspace, | ||
| name: path.basename(newLoc.fsPath), | ||
| uri: newLoc, | ||
| }; | ||
| } | ||
|
|
||
| // searches up the folder structure until it finds a Cargo.toml | ||
| export async function nearestParentWithCargoToml( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have this implemented in rust (
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not sure what it takes away from what is already implemented / what visions it's breaking?
That's a good point, I saw this as a very minor change to improve QoL for vscode, but I can see how it might be better to do 100% server side for many other reasons. It took me a couple of hours to code up (mostly reading vscode issues). If that's what is ruining this pr for you I could redo it in the server, however there it will actually affect all users of it (but maybe that is what we want?).
There are many use-cases
|
||
| workspaceRootUri: vscode.Uri, | ||
| fileLoc: vscode.Uri, | ||
| ): Promise<vscode.Uri | null> { | ||
| const fileExists: (path: fs.PathLike) => Promise<boolean> = util.promisify(fs.exists); | ||
| // check that the workspace folder already contains the "Cargo.toml" | ||
| const workspaceRoot = workspaceRootUri.fsPath; | ||
| // algorithm that will strip one folder at a time and check if that folder contains "Cargo.toml" | ||
| let current = fileLoc.fsPath; | ||
| if (fileLoc.fsPath.substring(0, workspaceRoot.length) !== workspaceRoot) { | ||
| return null; | ||
| } | ||
| while (true) { | ||
| const old = current; | ||
| current = path.dirname(current); | ||
|
|
||
| // break in case there is a bug that could result in a busy loop | ||
| if (old === current) { | ||
| break; | ||
| } | ||
|
|
||
| // break in case the strip folder reached the workspace root | ||
| if (workspaceRoot === current) { | ||
| break; | ||
| } | ||
|
|
||
| // check if "Cargo.toml" is present in the parent folder | ||
| const cargoPath = path.join(current, 'Cargo.toml'); | ||
| if (await fileExists(cargoPath)) { | ||
| // ghetto change the uri on Workspace folder to make vscode think it's located elsewhere | ||
| return vscode.Uri.file(current); | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
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.
Adding a command line option, given that this is the first command line option, massively changes the interface of the program. It's much better to stick to existing conventions, which, in this case, are the initializationOptions (not sure about the naming) field of the initialize requst
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.
As I said I couldn't find a way to send it via the initialization options in vscode. It also shouldn't be a massive change to the interface since it's a completely optional parameter but I can see how it feels intrusive.