Skip to content

Upgrade the LSP client library for better logging support #8281

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 8 additions & 4 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
"group": {
"kind": "build",
"isDefault": true
}
},
"problemMatcher": "$tsc-watch",
Copy link
Member

Choose a reason for hiding this comment

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

What was mandating this? And is tsc-watch correct even if we're not doing a live watch?

Copy link
Member Author

Choose a reason for hiding this comment

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

What was mandating this?

Me wanting errors to appear when I hit F5 and it failed to build.

And is tsc-watch correct even if we're not doing a live watch?

Ah good call. Will fix.

},
{
"label": "buildDev",
Expand All @@ -25,7 +26,8 @@
"group": {
"kind": "build",
"isDefault": true
}
},
"problemMatcher": "$tsc-watch",
},
{
"label": "package",
Expand All @@ -35,7 +37,8 @@
"group": {
"kind": "build",
"isDefault": true
}
},
"problemMatcher": "$tsc-watch",
},
{
"label": "packageDev",
Expand All @@ -45,7 +48,8 @@
"group": {
"kind": "build",
"isDefault": true
}
},
"problemMatcher": "$tsc-watch",
},
{
"label": "test",
Expand Down
108 changes: 52 additions & 56 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@
"tmp": "0.0.33",
"uuid": "^9.0.0",
"vscode-html-languageservice": "^5.3.1",
"vscode-jsonrpc": "9.0.0-next.7",
"vscode-languageclient": "10.0.0-next.14",
"vscode-jsonrpc": "9.0.0-next.8",
"vscode-languageclient": "10.0.0-next.15",
"vscode-languageserver-protocol": "3.17.6-next.12",
"vscode-languageserver-textdocument": "1.0.12",
"vscode-languageserver-types": "3.17.6-next.6",
Expand Down
5 changes: 2 additions & 3 deletions src/lsptoolshost/activate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { RazorLogger } from '../razor/src/razorLogger';
import { registerRazorEndpoints } from './razor/razorEndpoints';

let _channel: vscode.LogOutputChannel;
let _traceChannel: vscode.OutputChannel;
let _traceChannel: vscode.LogOutputChannel;

/**
* Creates and activates the Roslyn language server.
Expand All @@ -51,8 +51,7 @@ export async function activateRoslynLanguageServer(
// Create a channel for outputting general logs from the language server.
_channel = outputChannel;
// Create a separate channel for outputting trace logs - these are incredibly verbose and make other logs very difficult to see.
// The trace channel verbosity is controlled by the _channel verbosity.
_traceChannel = vscode.window.createOutputChannel(vscode.l10n.t('C# LSP Trace Logs'));
_traceChannel = vscode.window.createOutputChannel(vscode.l10n.t('C# LSP Trace Logs'), { log: true });
Copy link
Member

Choose a reason for hiding this comment

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

A slightly interesting question. Now that both of these are logger output windows, do we now have a separate log level configuration for the trace window?

Previously I manually tied it to the C# output window log level (by detecting changes to the level, and updating the LSP clients trace level), but maybe we shouldn't do that any more?

If so, we should remove the code that updates the trace log level based on the C# output window level, and update Support.md to mention how to get the trace lgos.

Copy link
Member Author

Choose a reason for hiding this comment

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

They both have a setting now yes. I was still noticing that it seemed I had to set the setting for the base one to also get the other one. Where is that code?

Copy link
Member

Choose a reason for hiding this comment

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


reporter.sendTelemetryEvent(TelemetryEventNames.ClientInitialize);

Expand Down
2 changes: 1 addition & 1 deletion src/lsptoolshost/server/roslynLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ export class RoslynLanguageServer {
additionalExtensionPaths: string[],
languageServerEvents: RoslynLanguageServerEvents,
channel: vscode.LogOutputChannel,
traceChannel: vscode.OutputChannel
traceChannel: vscode.LogOutputChannel
): Promise<RoslynLanguageServer> {
const devKit = getCSharpDevKit();
if (devKit) {
Expand Down
4 changes: 2 additions & 2 deletions src/omnisharp/engines/lspEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as ObservableEvents from '../omnisharpLoggingEvents';
import { EventStream } from '../../eventStream';
import CompositeDisposable from '../../compositeDisposable';
import Disposable from '../../disposable';
import { ExtensionContext, CancellationTokenSource, OutputChannel, Location, CodeLens, Uri } from 'vscode';
import { ExtensionContext, CancellationTokenSource, LogOutputChannel, Location, CodeLens, Uri } from 'vscode';
import { LanguageMiddlewareFeature } from '../languageMiddlewareFeature';
import { Events, OmniSharpServer } from '../server';
import { IEngine } from './IEngine';
Expand Down Expand Up @@ -42,7 +42,7 @@ export class LspEngine implements IEngine {
private eventBus: EventEmitter,
private eventStream: EventStream,
private context: ExtensionContext,
private outputChannel: OutputChannel,
private outputChannel: LogOutputChannel,
private disposables: CompositeDisposable,
private languageMiddlewareFeature: LanguageMiddlewareFeature,
private platformInfo: PlatformInformation,
Expand Down
8 changes: 7 additions & 1 deletion src/omnisharp/omnisharpLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,13 @@ export async function activateOmniSharpLanguageServer(
eventStream.subscribe(dotnetTestChannelObserver.post);
eventStream.subscribe(dotnetTestLoggerObserver.post);

const omnisharpChannel = vscode.window.createOutputChannel(vscode.l10n.t('OmniSharp Log'));
// If we're in LSP mode, we can create a LogOutputChannel since that's what the LSP client now supports.
// If we're not in LSP mode, we'll create a regular OutputChannel since the log formatting expects to be able to write
// it's own formatted outputs which gets mixed up with LogOutputChannels.s
const omnisharpChannel = omnisharpOptions.enableLspDriver
? vscode.window.createOutputChannel(vscode.l10n.t('OmniSharp Log'), { log: true })
: vscode.window.createOutputChannel(vscode.l10n.t('OmniSharp Log'));

const omnisharpLogObserver = new OmnisharpLoggerObserver(omnisharpChannel, platformInfo);
const omnisharpChannelObserver = new OmnisharpChannelObserver(omnisharpChannel);
eventStream.subscribe(omnisharpLogObserver.post);
Expand Down
5 changes: 3 additions & 2 deletions src/omnisharp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { Subject } from 'rxjs';
import { debounceTime } from 'rxjs/operators';
import CompositeDisposable from '../compositeDisposable';
import Disposable from '../disposable';
import { ExtensionContext, OutputChannel } from 'vscode';
import { ExtensionContext, LogOutputChannel, OutputChannel } from 'vscode';
import { LanguageMiddlewareFeature } from './languageMiddlewareFeature';
import { LspEngine } from './engines/lspEngine';
import { IEngine } from './engines/IEngine';
Expand Down Expand Up @@ -314,7 +314,8 @@ export class OmniSharpServer {
this._eventBus,
this.eventStream,
this.context,
this.outputChannel,
// If we are in LSP mode, then we created an LogOutputChannel originally
this.outputChannel as LogOutputChannel,
disposables,
this.languageMiddlewareFeature,
this.platformInfo,
Expand Down
2 changes: 1 addition & 1 deletion src/razor/src/razorLanguageServerOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as vscode from 'vscode';

export interface RazorLanguageServerOptions {
serverPath: string;
outputChannel?: vscode.OutputChannel;
outputChannel?: vscode.LogOutputChannel;
debug?: boolean;
usingOmniSharp: boolean;
forceRuntimeCodeGeneration: boolean;
Expand Down
Loading