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

Show progress during startup #92

Closed
gabro opened this issue Dec 3, 2017 · 13 comments
Closed

Show progress during startup #92

gabro opened this issue Dec 3, 2017 · 13 comments

Comments

@gabro
Copy link
Member

gabro commented Dec 3, 2017

Essentially this:

This can leverage the LSP notifications of type "status". The LS will send a notification indicating the status (Starting, Started, Error) and a message.

Each specific editor can decide how to show this message. The example in the GIF above is a mocked implementation in VSCode.

The Java LSP does it exactly like this:

@laughedelic
Copy link
Member

I'm not sure what is connection.sendStatus in that code and the "status" type of notifications you mention. window/logMessage has only 4 types of message (MessageType):

  • Error
  • Warning
  • Info
  • Log

We can use Log type, of course and then have a convention with clients to treat it specially, but I think this doesn't fit nicely with LSP.

@gabro
Copy link
Member Author

gabro commented Dec 3, 2017

Ah, I thought this was a standard lsp features but it looks like it's a custom addition by eclipse lsp? https://github.com/eclipse/eclipse.jdt.ls/blob/0bba050b76ea867564bd6221e78782bc69d09cfa/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java#L39

I'll look it up better tomorrow.

@laughedelic
Copy link
Member

This is how it looks now in Atom:

2017-12-03 22 48 54

No details about indexing, it just starts spinning when initialization request is sent and stops when it gets a response. And it works even without logMessage, because atom-languageclient allows you to hook on pre/post-initialization. I think vscode-languageclient should have something similar. Other requests usually don't take so long, so it's not essential.

I think that doing the same in VS Code client + implementing #85 will be enough: if you're wondering what exactly is happening, you can check logs. In VS Code there's a special panel for that, in Atom, unfortunately, it's still not implemented..

@gabro
Copy link
Member Author

gabro commented Dec 4, 2017

Ah right, without anything fancy, we could probably just use the onReady method on the client to show a spinner (without any details).

@cquiroz
Copy link
Contributor

cquiroz commented Dec 4, 2017

@laughedelic I don't see the spinner in Atom, is that change on the plugin?

@olafurpg
Copy link
Member

olafurpg commented Dec 4, 2017

I think it's OK to implement vscode specific functionality if it's valuable and not too much work. A progress bar would be super nice for example, it's one of thing I I'd otherwise miss from IntelliJ.

@gabro
Copy link
Member Author

gabro commented Dec 4, 2017

The problem is that we need to define some custom message over the protocol to communicate the progress from the server to the client. The Java LSP seems to do just that.

The editor-specific part is how to show the progress, but that's really tiny.

@ShaneDelmore
Copy link
Collaborator

This looks like it has at least some support from vscode team. Might be worth using until there is an official answer. microsoft/vscode-languageserver-node#261

@laughedelic
Copy link
Member

laughedelic commented Dec 5, 2017

@cquiroz Try pulling the master and running sbt apmPackage again. I forgot to push an update of atom-languageclient which adds this feature. If it doesn't work, ping me on Gitter, I'll help you to debug it.

I think it's OK to implement vscode specific functionality if it's valuable and not too much work.

As @gabro says, it's not so much about vscode specific functionality. LSP just doesn't support yet this concept of progress communication. I think we should wait for the linked microsoft/vscode-languageserver-node#261 (note, it's already a pull-request) instead of inventing conventions.

I think if @gabro knows how to do #92 (comment), it's a good compromise solution for now, similar to what I shown in Atom.

P.S. sorry, closed it accidentally

@olafurpg
Copy link
Member

olafurpg commented Jan 6, 2018

This should be fairly easy easier to do now after #167 and #164, we need to first detect if the client supports this feature and then send notifications to the "status" endpoint

@olafurpg
Copy link
Member

After #332, actual indexing of project and dependency sources + semanticdb may be fast enough that it won't be necessary to report progress. The source file indexer processes ~800k loc/s.

I think it's worth revisiting this however once we have a bsp client that triggers compile in the build tool to report batch compilation progress in the build tool.

@pca006132
Copy link

Hi, LSP now supports progress report via $/progress: https://microsoft.github.io/language-server-protocol/specifications/specification-current/#progress . Wondering if it is possible to add an option to statusBarProvider to use this method, so editor plugins such as https://github.com/j-hui/fidget.nvim can handle this without the need to special case metals.

@ckipp01
Copy link
Member

ckipp01 commented Mar 31, 2022

Hey @pca006132 we actually have a feature request for this here: scalameta/metals-feature-requests#125. We have a couple different custom LSP extension that we use in instead of progress to enable us to do a few more things, but it is a good idea to default to progress when the client doesn't support metals/status.

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

No branches or pull requests

7 participants