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

Don't start/attach language server for semi-transient views #2298

Open
jwortmann opened this issue Jul 14, 2023 · 5 comments
Open

Don't start/attach language server for semi-transient views #2298

jwortmann opened this issue Jul 14, 2023 · 5 comments
Labels
discussion sublime issue Issues related to shortcomings or bugs in the ST API

Comments

@jwortmann
Copy link
Member

This is an enhancement suggestion that I had in mind for a pretty long time already, but I think unfortunately it's not possible to implement with the current ST API. But perhaps someone has an opinion, even an idea for a trick or workaround, or wants to add anything else.

Terminology:

  • transient views1 are preview only - they don't have a tab (e.g. when you highlight entries in the "Goto Anything" quick panel).
  • semi-transient views do have a tab, but the tab will be replaced if you open another semi-transient view (e.g. single click on a file in the side bar with default "preview_on_click": true setting). And they are not shown in the "Open Files" section in the sidebar.

LSP currently tries to start/attach the language server when a normal (non transient, non semi-transient) view or a semi-transient view gets opened, see also is_regular_view. This can be annoying for semi-transient views if you just click trough some files in the side bar to preview (or via the arrow keys). Especially if a language server takes a long time to initialize (*hmpf*, Julia) - even though the process will get killed/shutdown a few seconds later when you switch to preview another file. Note that as soon as you edit the file, or when you double click on the tab of a semi-transient view, it will get automatically promoted to a regular view. This situation is when I would ideally start/attach the language server (for formerly semi-transient views).

There is also an API function to manually remove the (semi-)transient property of a view: Window.promote_sheet. But unfortunately there is no EventListener method that would be called when it happens, and I can't think of any other way how to get notified about it. It might be useful to create a feature request in the ST issue tracker in case there is agreement about this suggestion.

Footnotes

  1. actually it's a property of the sublime.Sheet, but it doesn't really matter here

@jwortmann jwortmann added enhancement sublime issue Issues related to shortcomings or bugs in the ST API labels Jul 14, 2023
@rchl
Copy link
Member

rchl commented Aug 2, 2023

I have never been bothered by the current behavior and probably would even miss this behavior if it wouldn't happen.

That said I don't run Julia :)

If you think that it would be useful then probably there are other people that would think so too. Besides the fact that it's not doable right now, how would you see this implemented? As a global or per-server option? Do you think the current behavior should be the default?

@jwortmann
Copy link
Member Author

Besides the fact that it's not doable right now, how would you see this implemented? As a global or per-server option? Do you think the current behavior should be the default?

I wouldn't even make it configurable. In my opinion it should be the only possible and expected behavior to not start the server for semi-transient views. Regarding implementation, just add and not v.sheet().is_semi_transient() to the condition of

LSP/plugin/documents.py

Lines 63 to 66 in 75bd043

def is_regular_view(v: sublime.View) -> bool:
# Not from the quick panel (CTRL+P), and not a special view like a console, output panel or find-in-files panels.
is_widget = v.settings().get('is_widget')
return not v.sheet().is_transient() and v.element() is None and not is_widget

and then use a potential new API method on_promote_sheet of EventListener to attach the language server (like currently in on_activated_async).

I'm a bit surprised that you like/prefer the current behavior, and I wonder if you would then prefer to attach the server to transient views too, like when scrolling through the Goto Anything quick panel? I mean they are both used as a file preview, and the only difference is that semi-transient sheets have a tab, while fully transient don't.

@rchl
Copy link
Member

rchl commented Aug 3, 2023

I wouldn't want/need to start server on previewing from Goto panel since its not at this point clear whether I want to even look at a given file.

But with semi-transient view, if it's my project then I'm pretty sure I've opened files like that before to check if there are any issues in the file after making other changes. Hard to say for sure as it's an automatic process and I don't think when I do that.

@jwortmann
Copy link
Member Author

jwortmann commented Sep 17, 2023

I had an idea for a compromise (in theory); if there is already a session running in the window, then the semi-transient sheets would get attached to this session. But if there is no applicable session, opening a semi-transient sheet won't initiate the start of a language server. This way diagnostics would still be visible when clicking through preview sheets after making other changes (assuming that you have some other file still open), but it can prevent an accidental start of a server for unrelated files.

Unfortunately still not possible to implement at the momemt, without having some kind of event triggered when a sheet gets promoted from semi-transient.


Not directly related, but I also wonder whether we could render diagnostics even for transient sheets. Because right now, when you use the "Goto Diagnostic in Project" panel, then the diagnostics are shown in the quick panel, but not in the file itself while being a preview, unless the file was already open. But I'm not sure how much refactoring would be needed to make the diagnostics available even when the view is not attached to the language server (I think at the moment rendering is bound to SessionView).

diagnostics.webm

@rchl
Copy link
Member

rchl commented Sep 19, 2023

Not directly related, but I also wonder whether we could render diagnostics even for transient sheets. Because right now, when you use the "Goto Diagnostic in Project" panel, then the diagnostics are shown in the quick panel, but not in the file itself while being a preview, unless the file was already open.

I think that would make sense. But it shouldn't apply just to transient files but to any files because server can technically also report diagnostics for files it doesn't support itself. For example typescript server can report diagnostics for its configuration file tsconfig.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion sublime issue Issues related to shortcomings or bugs in the ST API
Projects
None yet
Development

No branches or pull requests

2 participants