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

Replace slowTask with LSP progress #374

Closed
ckipp01 opened this issue Feb 13, 2024 · 2 comments · Fixed by scalameta/metals#6144
Closed

Replace slowTask with LSP progress #374

ckipp01 opened this issue Feb 13, 2024 · 2 comments · Fixed by scalameta/metals#6144
Assignees

Comments

@ckipp01
Copy link
Member

ckipp01 commented Feb 13, 2024

Is your feature request related to a problem? Please describe.

Custom server endpoints are something that should be avoided unless they are absolutely necessary. Now that #125 is implemented I think we can go even further to utilize it to completely remove the metals/slowTask.

Describe the solution you'd like

As stated in #125 one of the most important parts of metals/slowTask was the ability to cancel. In the beginning it looks like progress didn't support this, but looking at the spec here you'll notice:

	/**
	 * Controls if a cancel button should show to allow the user to cancel the
	 * long running operation. Clients that don't support cancellation are
	 * allowed to ignore the setting.
	 */
	cancellable?: boolean;

I think it's a worth goal to attempt to just replace metals/slowTask with this and get rid of it for good. That's one less extension that any client needs to implement and also provides a better experience for clients that don't have dedicated extensions.

The one hurdle we'd need to tackle is that currently LSP progress is a fallback and we don't use it by default but instead use the Metals status. I think this should really be reconsidered and explored. What exactly do we actually need the Metals status for, and is there anything it's doing that can't be done within the spec. Maybe a good way to start would be to stop using Metals status for any progress related things and fully use LSP progress for this. This would be an instant win for every client probably except for VS Code, which fully implements status. So we'd probably need to ensure that the changes made to accommodate this won't impact VS Code users negatively.

Describe alternatives you've considered

Do what we do now.

Additional context

image

Search terms

progress slowTask

@tgodzik
Copy link
Contributor

tgodzik commented Feb 15, 2024

I think the main current difference would be the additional status for build tools, but we can keep that and revert everything else to progress. @kasiaMarek was changing things around there recently, what do you think?

@kasiaMarek
Copy link

I think it's very reasonable and hopefully can simplify the logic of metals status. I'm not sure we can revert everything else to progress, since there are messages that we show on the status that are not slow tasks, but we can keep those as well.

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

Successfully merging a pull request may close this issue.

3 participants