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

not possible to un-ignore a file after ignoring through should_ignore #2503

Open
TerminalFi opened this issue Jul 15, 2024 · 6 comments
Open

Comments

@TerminalFi
Copy link
Contributor

TerminalFi commented Jul 15, 2024

Previously, LSP-Copilot had an open issue for how do we prevent a View/File from being managed by Github Copilot with disabling it on the entire syntax.

@jwortmann was kind enough to think this through and add the below function.

LSP/plugin/core/sessions.py

Lines 956 to 965 in 293f4a4

def should_ignore(cls, view: sublime.View) -> bool:
"""
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). Please note that this also means that no document syncronization
notifications (textDocument/didOpen, textDocument/didChange, textDocument/didClose, etc.) are sent to the server
for ignored views, when they are opened in the editor. Therefore this method should be used with caution for
language servers which index all files in the workspace.
"""
return False

However, now when we want to ignore a file, if that file is to ever be add back to the session, it requires asking the user to close and re-open the file or doing the below hack.

https://github.com/TerminalFi/LSP-copilot/blob/095fd059d1c6f7faaaedc75dc828ed83476d29f1/plugin/listeners.py#L61-L69

What this leads to is impacting other LSP sessions / plugins. See https://discord.com/channels/280102180189634562/1261340378133434439/1261340581779603497 for more details

Suggestion

Add another class func that can be used to trigger a view to be added to the LSP-* session

@rchl
Copy link
Member

rchl commented Jul 15, 2024

I suppose a correct solution would be for LSP to automatically unignore the file if should_ignore returns false. Having opposite method sounds either redundant or at least confusing.

And of course that would likely mean that LSP has to call this method more often than it does now.

@jfcherng
Copy link
Contributor

jfcherng commented Jul 15, 2024

I think the issue we have is that should_ignore is only called once for a view. LSP-copilot's expectation is that we can call should_ignore whenever a view is activated.

@rchl rchl changed the title should_ignore needs a counterpart should_unignore not possible to un-ignore the file after ignoring through should_ignore Jul 15, 2024
@rchl rchl changed the title not possible to un-ignore the file after ignoring through should_ignore not possible to un-ignore a file after ignoring through should_ignore Jul 15, 2024
@rchl
Copy link
Member

rchl commented Jul 15, 2024

On the other hand... it's hard for LSP to know exactly when LSP-* wants to un-ignore a file. It could be at arbitrary time which would then make it pretty much impossible for current solution to work as intended. Having a session method like reevaluate_should_ignore() might have to be necessary in this case.

@jfcherng
Copy link
Contributor

reevaluate_should_ignore() might have to be necessary in this case.

that would be the best 😄

@TerminalFi
Copy link
Contributor Author

Yes, reevaluate_should_ignore seems like a great solution

@TerminalFi
Copy link
Contributor Author

Due to the hacky way we did it initially. It was causing issues in other LSP plugins. So this is now more relevant

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

No branches or pull requests

3 participants