search

sublimelsp / LSP

Client implementation of the Language Server Protocol for Sublime Text
https://lsp.sublimetext.io/
MIT License
1.65k stars 182 forks source link

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

Open TerminalFi opened 3 months ago

TerminalFi commented 3 months ago

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.

https://github.com/sublimelsp/LSP/blob/293f4a4340cca5ab1ad065643e4f20d9b270b2b1/plugin/core/sessions.py#L956-L965

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

that would be the best 😄

TerminalFi commented 3 months ago

Yes, reevaluate_should_ignore seems like a great solution

TerminalFi commented 3 months ago

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