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.

Sign up for GitHub

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

Allow plugins to ignore certain views #2410

Merged
merged 4 commits into from
Mar 10, 2024
Merged

Allow plugins to ignore certain views #2410

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
813c7ad
Allow plugins to ignore certain views
jwortmann Jan 31, 2024
a236380
Fix preventing start of all sessions if one session ignores view
jwortmann Feb 2, 2024
61f799e
Merge branch 'main' into plugin-ignore-uri
jwortmann Feb 3, 2024
64b33a4
Try to fix it
jwortmann Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions plugin/core/sessions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,15 @@ def on_post_start(cls, window: sublime.Window, initiating_view: sublime.View,
"""
pass

@classmethod
def should_ignore(cls, view: sublime.View) -> bool:
"""
Allows to exclude a view from being handled by the language server, even if it matches the URI scheme(s) and
selector from the configuration. This can be used to, for example, ignore certain file patterns which are listed
in a configuration file (e.g. .gitignore).
"""
return False

@classmethod
def markdown_language_id_to_st_syntax_map(cls) -> Optional[MarkdownLangMap]:
"""
Expand Down Expand Up @@ -1382,6 +1391,9 @@ def compare_by_string(sb: Optional[SessionBufferProtocol]) -> bool:
def can_handle(self, view: sublime.View, scheme: str, capability: Optional[str], inside_workspace: bool) -> bool:
if not self.state == ClientStates.READY:
return False
if self._plugin and self._plugin.should_ignore(view):
debug(view, "ignored by plugin", self._plugin.__class__.__name__)
return False
Comment on lines +1394 to +1396
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is the best place here for this check, because the can_handle methods sounds like it might be called more often than necessary. Though the only other reference is in WindowManager.sessions, but that seems to be an unused method (?)

if scheme == "file":
file_name = view.file_name()
if not file_name:
Expand Down
6 changes: 6 additions & 0 deletions plugin/core/windows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ def disable_config_async(self, config_name: str) -> None:
self._config_manager.disable_config(config_name)

def register_listener_async(self, listener: AbstractViewListener) -> None:
config = self._needed_config(listener.view)
if config:
plugin = get_plugin(config.name)
if plugin and plugin.should_ignore(listener.view):
debug(listener.view, "ignored by plugin", plugin.__name__)
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this part specifically but it can get into an infinite loop of starting and closing the server. I think when the first opened file is ignored.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example where you have seen such an infinite loop? It seems to work as expected on my side, even when the first opened file is ignored.

Copy link
Member

@rchl rchl Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be a bit random.
I've modified LSP-pyright to ignore all files in its plugin.py and then restarted LSP.
Also note that I have 3 different servers starting on that file (LSP-pylsp, LSP-ruff and LSP-pyright).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could reproduce it with multiple sessions. This code was indeed misplaced here, because it would disable the listener entirely if any of the applicable sessions ignores the view. I don't know why this did lead to the infinite starting loop, but it's somewhat difficult to understand (for me) what exactly happens in the _dequeue_listener_async method, which calls itself recursively. I think the check for ignored views is in the right place now, and this bug should be fixed now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still see this problem.

After adding this to LSP-pyright:

    @classmethod
    def should_ignore(cls, view: sublime.View) -> bool:
        return True

None of the sessions are starting anymore on LSP-pyright/plugin.py. I'm expecting LSP-pylsp and LSP-ruff to still be able to start.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed to work randomly for me, sometimes it worked and sometimes it didn't. Could you try again with the latest changes?

set_diagnostics_count(listener.view, self.total_error_count, self.total_warning_count)
# Update workspace folders in case the user have changed those since window was created.
# There is no currently no notification in ST that would notify about folder changes.
Expand Down