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.

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

Feature request: mechanism for intercepting progress reports #671

Closed
hyangah opened this issue Sep 15, 2020 · 9 comments
Closed

Feature request: mechanism for intercepting progress reports #671

hyangah opened this issue Sep 15, 2020 · 9 comments
Labels
help wanted Issues identified as good community contribution opportunities

Comments

@hyangah
Copy link
Contributor

hyangah commented Sep 15, 2020

I want to intercept the progress messages sent from the server, but don't see how yet.
Other LSP messages, Middleware in LanguageClientOptions provides the functionality.

export interface _Middleware {
didOpen?: NextSignature<TextDocument, void>;
didChange?: NextSignature<TextDocumentChangeEvent, void>;
willSave?: NextSignature<TextDocumentWillSaveEvent, void>;
willSaveWaitUntil?: NextSignature<TextDocumentWillSaveEvent, Thenable<VTextEdit[]>>;
didSave?: NextSignature<TextDocument, void>;
didClose?: NextSignature<TextDocument, void>;
handleDiagnostics?: (this: void, uri: Uri, diagnostics: VDiagnostic[], next: HandleDiagnosticsSignature) => void;
provideCompletionItem?: (this: void, document: TextDocument, position: VPosition, context: VCompletionContext, token: CancellationToken, next: ProvideCompletionItemsSignature) => ProviderResult<VCompletionItem[] | VCompletionList>;
resolveCompletionItem?: (this: void, item: VCompletionItem, token: CancellationToken, next: ResolveCompletionItemSignature) => ProviderResult<VCompletionItem>;
provideHover?: (this: void, document: TextDocument, position: VPosition, token: CancellationToken, next: ProvideHoverSignature) => ProviderResult<VHover>;
provideSignatureHelp?: (this: void, document: TextDocument, position: VPosition, context: VSignatureHelpContext, token: CancellationToken, next: ProvideSignatureHelpSignature) => ProviderResult<VSignatureHelp>;
provideDefinition?: (this: void, document: TextDocument, position: VPosition, token: CancellationToken, next: ProvideDefinitionSignature) => ProviderResult<VDefinition | VDefinitionLink[]>;
provideReferences?: (this: void, document: TextDocument, position: VPosition, options: { includeDeclaration: boolean; }, token: CancellationToken, next: ProvideReferencesSignature) => ProviderResult<VLocation[]>;
provideDocumentHighlights?: (this: void, document: TextDocument, position: VPosition, token: CancellationToken, next: ProvideDocumentHighlightsSignature) => ProviderResult<VDocumentHighlight[]>;
provideDocumentSymbols?: (this: void, document: TextDocument, token: CancellationToken, next: ProvideDocumentSymbolsSignature) => ProviderResult<VSymbolInformation[] | VDocumentSymbol[]>;
provideWorkspaceSymbols?: (this: void, query: string, token: CancellationToken, next: ProvideWorkspaceSymbolsSignature) => ProviderResult<VSymbolInformation[]>;
provideCodeActions?: (this: void, document: TextDocument, range: VRange, context: VCodeActionContext, token: CancellationToken, next: ProvideCodeActionsSignature) => ProviderResult<(VCommand | VCodeAction)[]>;
provideCodeLenses?: (this: void, document: TextDocument, token: CancellationToken, next: ProvideCodeLensesSignature) => ProviderResult<VCodeLens[]>;
resolveCodeLens?: (this: void, codeLens: VCodeLens, token: CancellationToken, next: ResolveCodeLensSignature) => ProviderResult<VCodeLens>;
provideDocumentFormattingEdits?: (this: void, document: TextDocument, options: VFormattingOptions, token: CancellationToken, next: ProvideDocumentFormattingEditsSignature) => ProviderResult<VTextEdit[]>;
provideDocumentRangeFormattingEdits?: (this: void, document: TextDocument, range: VRange, options: VFormattingOptions, token: CancellationToken, next: ProvideDocumentRangeFormattingEditsSignature) => ProviderResult<VTextEdit[]>;
provideOnTypeFormattingEdits?: (this: void, document: TextDocument, position: VPosition, ch: string, options: VFormattingOptions, token: CancellationToken, next: ProvideOnTypeFormattingEditsSignature) => ProviderResult<VTextEdit[]>;
provideRenameEdits?: (this: void, document: TextDocument, position: VPosition, newName: string, token: CancellationToken, next: ProvideRenameEditsSignature) => ProviderResult<VWorkspaceEdit>;
prepareRename?: (this: void, document: TextDocument, position: VPosition, token: CancellationToken, next: PrepareRenameSignature) => ProviderResult<VRange | { range: VRange, placeholder: string }>;
provideDocumentLinks?: (this: void, document: TextDocument, token: CancellationToken, next: ProvideDocumentLinksSignature) => ProviderResult<VDocumentLink[]>;
resolveDocumentLink?: (this: void, link: VDocumentLink, token: CancellationToken, next: ResolveDocumentLinkSignature) => ProviderResult<VDocumentLink>;
executeCommand?: (this: void, command: string, args: any[], next: ExecuteCommandSignature) => ProviderResult<any>;
workspace?: WorkspaceMiddleware;
}
export type Middleware = _Middleware & TypeDefinitionMiddleware & ImplementationMiddleware & ColorProviderMiddleware &

Background: we want to enhance our integration tests by monitoring the progress reports from the language server, and ensuring the language server's internal state to be ready for probing/testing. If there is other way to achieve this, that's also appreciated.

@dbaeumer
Copy link
Member

@hyangah this is currently not possible. Two ideas:

  • we allow to access a set handler so that you can chain them
  • we add middleware support for this

A PR would be welcome.

@dbaeumer dbaeumer added this to the Backlog milestone Sep 16, 2020
@dbaeumer dbaeumer added the help wanted Issues identified as good community contribution opportunities label Sep 16, 2020
@DanTup
Copy link
Contributor

DanTup commented Oct 13, 2020

@dbaeumer which option do you prefer, and can you give me some pointers? I'm trying to port some of my tests to LSP, and discovered that I can't get at the progress events (if I use this.client.onProgress, it complains that they're already being handled - I presume by the code that reports the progress to VS Code).

@dbaeumer
Copy link
Member

@DanTup thanks for offering help. The right approach is a middle ware handler along the lines of the handleDiagnostics middleware since progress flows from the server to the client as well.

@DanTup
Copy link
Contributor

DanTup commented Oct 26, 2020

@dbaeumer does this seem like the right idea?

DanTup@5812e67

I'm not sure how to simulate the progress from the server in the test though - I couldn't find a similar test to copy (I couldn't find any diagnostic one, for example).

@dbaeumer
Copy link
Member

Added a comment here: DanTup@5812e67#commitcomment-43610915

@DanTup
Copy link
Contributor

DanTup commented Nov 3, 2020

@dbaeumer in case you didn't see it, I responded here with a question or two:

DanTup@5812e67#commitcomment-43611962

@dbaeumer
Copy link
Member

dbaeumer commented Nov 3, 2020

Sorry, too many things on my plate. I commented in the PR.

gopherbot pushed a commit to golang/vscode-go that referenced this issue Nov 10, 2020
When we wrote the first gopls integration test, gopls required
vscode to open the workspace folder while vscode code does not
support dynamic workspace folder registration in test mode. As a
result, we ended up having a fixed workspace folder as a test fixture,
starting the test code instance with the folder, and copying
necessary files to the folder. There were so many moving parts
and this created race conditions and caused slow test run.

Since v0.4.x, gopls starts to support single file edit and automatically
constructs an internal workspace by walking the directory tree
around the open file. This CL utilizes this new capability.
In each suite, we start testing by starting a new gopls, opening
a single file, and waiting for the gopls to finish the initial package
loading.

This CL introduces Env.onPatternInTrace, which watches the
fake trace output channel, and emits an event when a registered
pattern is observed. This is a hack to wait for the gopls's internal
state to reach to a desirable state - ideally, we want to intercept
all the progress report messages like gopls' regression tests
once it is supported from the language client middleware.
microsoft/vscode-languageserver-node#671

We also identified subtle issues in the existing test setup.

Issue 1: when the code for testing starts (using `vscode-test`'s
`runTests`) we pass the extension development path. It seems like
the vscode instance uses the `main` program specified in `package.json`
and activates it even without us asking. As a result, when we run
tests and call 'activate' again, multiple hover/completion providers
are registered, and vscode returns results from legacy and gopls-based
providers. For example, the completion middleware test was observing
entries from gopls, and goCompletionItemProvider that uses gocode.

We address this issue here by introducing the VSCODE_GO_IN_TEST
environment variable. If it is set, activate will return immediately.
So, tests can control when to register what, and how.

We need this setting in both `launch.json` and `runTest.ts` that's
invoked in CI (`npm run test`)

Issue 2: when the code for testing needs to call `activate`, we
got the extension instance by using `vscode.extensions.getExtension`
and called its `activate`. This was because there is no easy way
to supply sufficiently complete vscode's ExtensionContext.
It turned out, the extension instance vscode.extensions.getExtension
returns is the one built with the `main` program specified in
our `package.json` - that is the webpack'ed one in `dist/goMain.js`.
On the other hand, our debugging depends on pre-webpack versions
in `out/*`. This caused confusion and made debugging near impossible
(breakpoints we set on pre-webpack versions will not be hit because
we are running a different version of extension)!

We don't know if there is a way to teach `vscode-test` to use pre-webpack
version. Maybe this is our misconfiguration in our launch.json and
package.json. For now, we work around this issue by not calling
`activate`. Instead, in this gopls test, we call `buildLanguageClient`
directly. This required some refactoring work in goLanguageServer.ts.

Issue 3: sinon is cool, but stubbing vscode API for channel creation
is too much. While we refactor buildLanguageClient, we made changes
to let the caller supply the output channels.

Issue 4: as `vscode-test` starts the test vscode instance, it also
activates the registered snippets and it interferes with our gopls
completion middleware tests. In test, now we explicitly filter out
the snippet entries.

Issue 5: for some reason, the first entry in the completion middleware
test that expects 'Print' item, the filter text is not set. It can be
a bug, or working as intended (the first item has label === filterText).
Gopls is providing the expected entry. Workaround this issue by inspecting
the label field too.

Updates #655
Updates #832

Change-Id: Ic7088fd551329d1c8f78078ccb24a5f529eec72a
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/266418
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@ericdallo
Copy link

ericdallo commented Oct 3, 2021

@dbaeumer @DanTup I just realized that for some reason handleWorkDoneProgress is not called if server sends a $/progress notification during server intialize request, do you know why?
I tried using client.onProgress(...) as well but I get a error that the client is not ready yet, but AFAIK it's possible to server send $/progress notifications during initialize.

@dbaeumer
Copy link
Member

dbaeumer commented Oct 4, 2021

@ericdallo this is currently not possible due to the limitation how the language client handles progress on initialize. You can open a issue to request that support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues identified as good community contribution opportunities
Projects
None yet
Development

No branches or pull requests

4 participants