Allow plugins to ignore certain views

[jwortmann]

This makes it possible for plugins like LSP-copilot to ignore certain files (i.e. neither start the language server for files that should be ignored, nor attach them to a running session). It is implemented as a plugin API method because it may only be possible to decide this programmatically (e.g. read from a .copilotignore file).

[jwortmann]

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 (?)

[rchl]

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.

[jwortmann]

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.

[rchl]

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).

[jwortmann]

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.

[rchl]

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.

[jwortmann]

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

[netlify]

✅ Deploy Preview for sublime-lsp ready!

Name	Link
🔨 Latest commit	64b33a4
🔍 Latest deploy log	https://app.netlify.com/sites/sublime-lsp/deploys/65e9f9741d90d40008c7a7e8
😎 Deploy Preview	https://deploy-preview-2410--sublime-lsp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[rchl]

Forgot about it. Will try to have another look soon.

[jwortmann]

I was thinking a little bit more about this, and now I wonder whether this is really the right approach for ignored files. While I assume that it could work mostly okay in practice (and Copilot is probably the only server which would make use of it, because other servers shouldn't send file contents over the internet), there might be a problem that we didn't consider yet. If the language server isn't attached to the ignored files, it also means that no didOpen / didChange / didClose notifications for those files are sent to the server. But these are ownership notifications, i.e. didOpen means that the server isn't allowed anymore to read the file contents from disk. Extract from the LSP specs for didOpen:

The document open notification is sent from the client to the server to signal newly opened text documents. The document’s content is now managed by the client and the server must not try to read the document’s content using the document’s Uri. Open in this sense means it is managed by the client. It doesn’t necessarily mean that its content is presented in an editor.

And as mentioned in TerminalFi/LSP-copilot#126 (comment), servers might load the content of all unopened files in the workspace from disk anyway. So this means that

1. we can't really guarantee that the server doesn't read any confidential information itself.
2. without the notifications, a server might read the content of an ignored file from disk, for example to know the definition of a class in (an ignored) file A, in order to show the method completions of the class when it is imported/used in a (not ignored) file B. So this could even lead to the case where the completions are outdated, because the definition of the class in file A was changed in the editor in the meanwhile, but not saved to disk (or even if it was saved, but the server did read an earlier version of the file and never get any didSave notification).

Perhaps a better solution would be to still send didOpen, didChange, didClose, didSave notifications, but to never send any requests for the ignored files. On the other hand, this would explicitly send the content of the files to the server, defeating the purpose to keep it secret. And I think that LSP-copilot has a custom implementation for the completion requests to the server, so if there is a session available for the ignored files, it would still need some custom logic to inhibit its own requests.

I think this case of files with confidential information which should not become known to the server is just not considered in the LSP specs, and there isn't any good solution for it. Maybe this should be brought up to discussion in the specs repo first.

cc @rchl

[rchl]

It seems to me like a bit of a hypothetical problem since we don't know what the server is doing in either case.
And in practice it could do whatever it wants anyway, regardless of what we do.
So I'm not sure there is any point in changing anything more.

[jwortmann]

Well, my main point was that having this API could potentially make things worse for not-ignored files, because the language server might get out of sync with the content of the other files in the workspace. I don't know if Copilot indexes all workspace files in order to have more context for its completions, but iirc some AI tools advertise that they have the ability to analyze a whole project for improved context awareness. But for Copilot there is probably no way to find out what happens under the hood, because it's not open source but comes as a precompiled binary.

I guess we can try it for now, but should keep in mind that this functionality could have unintended side effects, and possibly revert it if we see that it causes problems.