pygls icon indicating copy to clipboard operation
pygls copied to clipboard

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

Open karthiknadig opened this issue 2 years ago • 4 comments

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.

karthiknadig avatar May 20 '22 02:05 karthiknadig

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 avatar Jul 07 '22 13:07 perrinjerome

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.

perrinjerome avatar Jul 17 '22 12:07 perrinjerome

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

karthiknadig avatar Jul 18 '22 18:07 karthiknadig

Thank you @karthiknadig

perrinjerome avatar Jul 18 '22 21:07 perrinjerome

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?

tombh avatar Dec 03 '22 20:12 tombh

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 )

perrinjerome avatar Jun 04 '23 14:06 perrinjerome