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

_WindowMiddleware - wrong definition for showDocument #1488

Closed
ljw1004 opened this issue May 30, 2024 · 1 comment · Fixed by #1489
Closed

_WindowMiddleware - wrong definition for showDocument #1488

ljw1004 opened this issue May 30, 2024 · 1 comment · Fixed by #1489

Comments

@ljw1004
Copy link

ljw1004 commented May 30, 2024

_WindowMiddleware.showDocument request from server to client is missing a "token" parameter. Here is the definition. Observe that the "next" parameter is a function which takes (params: ShowDocumentParams, token: CancellationToken). However, showDocument itself doesn't take a token parameter. Therefore we have nothing to pass when we invoke next(param, ???needToken??).

interface _WindowMiddleware {
showDocument?: (this: void, params: ShowDocumentParams, next: ShowDocumentRequest.HandlerSignature) => Promise<ShowDocumentResult>;
}

export namespace ShowDocumentRequest {
export const method: 'window/showDocument' = 'window/showDocument';
export const messageDirection: MessageDirection = MessageDirection.serverToClient;
export const type = new ProtocolRequestType<ShowDocumentParams, ShowDocumentResult, void, void, void>(method);
export type HandlerSignature = RequestHandler<ShowDocumentParams, ShowDocumentResult, void>;
export type MiddlewareSignature = (params: ShowDocumentParams, next: HandlerSignature) => HandlerResult<ShowDocumentResult, void>;
}

export interface RequestHandler<P, R, E> {
(params: P, token: CancellationToken): HandlerResult<R, E>;
}

By comparison, let's look at ConfigurationMiddleware.configuration request from server to client, where the signature of configuration does correctly take a token and is therefore able to pass it to the next function.

export interface ConfigurationMiddleware {
configuration?: ConfigurationRequest.MiddlewareSignature;
}

export namespace ConfigurationRequest {
export const method: 'workspace/configuration' = 'workspace/configuration';
export const messageDirection: MessageDirection = MessageDirection.serverToClient;
export const type = new ProtocolRequestType<ConfigurationParams, LSPAny[], never, void, void>(method);
export type HandlerSignature = RequestHandler<ConfigurationParams, LSPAny[], void>;
export type MiddlewareSignature = (params: ConfigurationParams, token: CancellationToken, next: HandlerSignature) => HandlerResult<LSPAny[], void>;
}

@ljw1004
Copy link
Author

ljw1004 commented May 30, 2024

I also think the return type of showDocument is wrong. It should allow the possibility to return error, since the next function is able to return error.

export type HandlerResult<R, E> = R | ResponseError<E> | Thenable<R> | Thenable<ResponseError<E>> | Thenable<R | ResponseError<E>>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant