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

Tell the language server to open projects if no solution exists #6062

Merged
merged 4 commits into from
Aug 8, 2023
Merged

Tell the language server to open projects if no solution exists #6062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fef49bc
Tell the language server to open projects if no solution exists
jasonmalinowski Aug 7, 2023
bd275d5
Move openSolutionParams and openProjectParams into roslynProtocol.ts
jasonmalinowski Aug 8, 2023
b8c8d66
Merge branch 'main' into load-projects-without-solution
dibarbet Aug 8, 2023
f84fd44
update server
arunchndr Aug 8, 2023
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
}
},
"defaults": {
"roslyn": "4.8.0-1.23407.6",
"roslyn": "4.8.0-1.23407.9",
"omniSharp": "1.39.7",
"razor": "7.0.0-preview.23405.1",
"razorOmnisharp": "7.0.0-preview.23363.1"
Expand Down
10 changes: 0 additions & 10 deletions src/lsptoolshost/openSolutionParams.ts
View file
Open in desktop

This file was deleted.

61 changes: 45 additions & 16 deletions src/lsptoolshost/roslynLanguageServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import ShowInformationMessage from '../shared/observers/utils/showInformationMes
import EventEmitter = require('events');
import Disposable from '../disposable';
import * as RoslynProtocol from './roslynProtocol';
import { OpenSolutionParams } from './openSolutionParams';
import { CSharpDevKitExports } from '../csharpDevKitExports';
import { ISolutionSnapshotProvider, SolutionSnapshotId } from './services/ISolutionSnapshotProvider';
import { Options } from '../shared/options';
Expand Down Expand Up @@ -115,6 +114,9 @@ export class RoslynLanguageServer {
*/
private _solutionFile: vscode.Uri | undefined;

/** The project files previously opened; we hold onto this for the same reason as _solutionFile. */
private _projectFiles: vscode.Uri[] = new Array<vscode.Uri>();
Copy link
Member

Choose a reason for hiding this comment

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

I understand the solution file is also a moving target to cache. But the contents were cache agnostic. The project file list on the other hand as a cache seems more volatile if cache and not tracked continuously.

Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker, just want to get on the same page

This comment was marked as resolved.

Sign in to view


constructor(
private platformInfo: PlatformInformation,
private hostExecutableResolver: IHostExecutableResolver,
Expand Down Expand Up @@ -182,10 +184,10 @@ export class RoslynLanguageServer {
this._languageClient.onDidChangeState(async (state) => {
if (state.newState === State.Running) {
await this._languageClient!.setTrace(languageClientTraceLevel);
if (this._solutionFile) {
await this.sendOpenSolutionNotification();
if (this._solutionFile || this._projectFiles.length > 0) {
await this.sendOpenSolutionAndProjectsNotifications();
} else {
await this.openDefaultSolution();
await this.openDefaultSolutionOrProjects();
}
await this.sendOrSubscribeForServiceBrokerConnection();
this._eventBus.emit(RoslynLanguageServer.serverStateChangeEvent, ServerStateChange.Started);
Expand Down Expand Up @@ -372,21 +374,37 @@ export class RoslynLanguageServer {

public async openSolution(solutionFile: vscode.Uri): Promise<void> {
this._solutionFile = solutionFile;
await this.sendOpenSolutionNotification();
this._projectFiles = [];
await this.sendOpenSolutionAndProjectsNotifications();
}

private async sendOpenSolutionNotification(): Promise<void> {
if (
this._solutionFile !== undefined &&
this._languageClient !== undefined &&
this._languageClient.isRunning()
) {
const protocolUri = this._languageClient.clientOptions.uriConverters!.code2Protocol(this._solutionFile);
await this._languageClient.sendNotification('solution/open', new OpenSolutionParams(protocolUri));
public async openProjects(projectFiles: vscode.Uri[]): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

do we have a picker for projects like we do slns?

Copy link
Member

Choose a reason for hiding this comment

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

I realize after I write this, a project picker for projects may not make sense, especially in huge workspaces. Though I am curious - what happens if we load a project that depends on another project via project reference (and the dependency is not loaded)?

Copy link
Member Author

Choose a reason for hiding this comment

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

@dibarbet I'm not including a picker in this PR. Omnisharp did have one but in the case of a large workspace I'm not entirely sure how useful that experience would have been, and at least one customer so far has pointed out they were switching projects when they probably didn't mean to.

If the dependency isn't there, we'll just treat it as a metadata reference and load the DLL.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. Lets make sure that experience is tracked as well. I agree it could be complicated - for example it would be nice if you once you select the projects to load you can save it somewhere (e.g. similar to default solution). Or maybe instead we just implement load projects on demand... Not sure either.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd hope projects-on-demand meets many needs, or yes, some other way to specify the list of projects (probably via globbing.)

this._solutionFile = undefined;
this._projectFiles = projectFiles;
await this.sendOpenSolutionAndProjectsNotifications();
}

private async sendOpenSolutionAndProjectsNotifications(): Promise<void> {
if (this._languageClient !== undefined && this._languageClient.isRunning()) {
if (this._solutionFile !== undefined) {
const protocolUri = this._languageClient.clientOptions.uriConverters!.code2Protocol(this._solutionFile);
await this._languageClient.sendNotification(RoslynProtocol.OpenSolutionNotification.type, {
solution: protocolUri,
});
}

if (this._projectFiles.length > 0) {
const projectProtocolUris = this._projectFiles.map((uri) =>
this._languageClient!.clientOptions.uriConverters!.code2Protocol(uri)
);
await this._languageClient.sendNotification(RoslynProtocol.OpenProjectNotification.type, {
projects: projectProtocolUris,
});
}
}
}

private async openDefaultSolution(): Promise<void> {
private async openDefaultSolutionOrProjects(): Promise<void> {
const options = this.optionProvider.GetLatestOptions();

// If Dev Kit isn't installed, then we are responsible for picking the solution to open, assuming the user hasn't explicitly
Expand All @@ -401,8 +419,19 @@ export class RoslynLanguageServer {
} else {
// Auto open if there is just one solution target; if there's more the one we'll just let the user pick with the picker.
const solutionUris = await vscode.workspace.findFiles('**/*.sln', '**/node_modules/**', 2);
if (solutionUris && solutionUris.length === 1) {
this.openSolution(solutionUris[0]);
if (solutionUris) {
if (solutionUris.length === 1) {
this.openSolution(solutionUris[0]);
} else if (solutionUris.length === 0) {
// We have no solutions, so we'll enumerate what project files we have and just use those.
const projectUris = await vscode.workspace.findFiles(
'**/*.csproj',
'**/node_modules/**',
options.omnisharpOptions.maxProjectResults
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an omnisharp option that I'm consuming here even on the non-omnisharp side, in case there is a user out there where this limit is significant. Having this not be a dotnet option is a bit of a violation of naming though, but I'm not sure I want to create a new option and setup the migration at this moment until we have a chance to spec it a bit more. Put another way, I'm OK with this bug at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

Fine with as-is - lets have an issue to track. But it should be pretty straightforward to migrate the option name if you wanted. We just put the O# name as the fallback, then add a value to https://github.com/dotnet/vscode-csharp/blob/main/src/shared/migrateOptions.ts

Copy link
Member Author

Choose a reason for hiding this comment

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

To echo something I mentioned to @dibarbet privately: making the fallback work is easy, but the option is a bit funky in this case since the filtering isn't exactly deterministic in what it will filter out: it prevents certain large repositories from causing pathological problems, but it's not ideal.

);

this.openProjects(projectUris);
}
}
}
}
Expand Down
20 changes: 20 additions & 0 deletions src/lsptoolshost/roslynProtocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@ export interface DebugAttachResult {
didAttach: boolean;
}

export interface OpenSolutionParams {
solution: lsp.DocumentUri;
}

export interface OpenProjectParams {
projects: lsp.DocumentUri[];
}

export interface ShowToastNotificationParams {
messageType: lsp.MessageType;
message: string;
Expand Down Expand Up @@ -172,3 +180,15 @@ export namespace DebugAttachRequest {
export const messageDirection: lsp.MessageDirection = lsp.MessageDirection.serverToClient;
export const type = new lsp.RequestType<DebugAttachParams, DebugAttachResult, void>(method);
}

export namespace OpenSolutionNotification {
export const method = 'solution/open';
export const messageDirection: lsp.MessageDirection = lsp.MessageDirection.clientToServer;
export const type = new lsp.NotificationType<OpenSolutionParams>(method);
}

export namespace OpenProjectNotification {
export const method = 'project/open';
export const messageDirection: lsp.MessageDirection = lsp.MessageDirection.clientToServer;
export const type = new lsp.NotificationType<OpenProjectParams>(method);
}