search

openlawlibrary / pygls

A pythonic generic language server
https://pygls.readthedocs.io/en/latest/
Apache License 2.0
588 stars 105 forks source link

`window/workDoneProgress/cancel` is not according to spec #233

Closed karthiknadig closed 1 year ago

karthiknadig commented 2 years ago

Looking at this: https://github.com/openlawlibrary/pygls/blob/4772250f8ede25aee8e457dae5bca394aeccdd2f/pygls/progress.py#L62-L66

From the spec it looks like the window/workDoneProgress/cancel is a notification that comes from client, and not a request that is sent by the server.

perrinjerome commented 2 years ago

This is also my understanding that this does not following the spec.

Also, at a higher level, I don't see how this implementation of cancel works. An API that would make sense to me would be is_cancelled() -> bool or on_cancel(callback)

perrinjerome commented 2 years ago

An API that would make sense to me would be is_cancelled() -> bool or on_cancel(callback)

I submitted a pull request ( #253 ) which implements this. The API is a future that is cancelled when window/workDoneProgress/cancel notification is received. If you have any feedback it would be appreciated, thanks.

karthiknadig commented 2 years ago

@perrinjerome You changes look good, thanks for working on this.

perrinjerome commented 2 years ago

Thank you @karthiknadig

tombh commented 1 year ago

I'm just wondering how the new v1 release affects this issue? I think there are 2 problems identified here. Is the first one, that window/workDoneProgress/cancel is a client-to-server notification, fixed?

perrinjerome commented 1 year ago

Is the first one, that window/workDoneProgress/cancel is a client-to-server notification, fixed?

yes it's fixed in #253 as well, the problem is that this cancel method sends a notification to client from server, which does not make sense. In #253 the method is removed, which solves the problem.

( I'm sorry I did not notice this question earlier )