TerminalFi / LSP-copilot

GitHub Copilot support for Sublime Text LSP plugin provided through Copilot.vim.
625 stars 25 forks source link

No Copilot suggestions if same file is opened in a separate pane (i.e. Sublime "column")? #102

Closed gregsadetsky closed 1 year ago

gregsadetsky commented 1 year ago

See description and repro video below :-)

When the same file is opened in separate pane (I know that it feels weird -- but sometimes I forget that a file is a tab in one pane, and I open it again in a different pane), Copilot suggestions don't appear. It seems that suggestions only appear if the file is opened once.

https://user-images.githubusercontent.com/1017304/233140718-5cca3d99-3fc4-4372-b76a-a3ac2cb3a6ff.mov

jfcherng commented 1 year ago

We (me, and @TerminalFi) discussed this before in a private chat. We didn't think this is solvable due to ST's API limitation.


This was the conclusion made by me:

They actually have different view ids. I try to add

    @classmethod
    def applies_to_primary_view_only(cls) -> bool:
        return False

but now both views sent a payload I didn't find something like "active_buffer()" from API docs. it's either

but both have their own issues

gregsadetsky commented 1 year ago

Super interesting! I wonder if a warning could be displayed..? That might be less confusing than not seeing Copilot (it took me a second to understand what was going on - since I had traced the issue before, I got it, but it could be confusing to someone who doesn't know?)

TerminalFi commented 1 year ago

It's something we are still considering. We might need to open a FR with Sublime.

jfcherng commented 1 year ago

Hmmm... seems get a working fix but I am not sure whether it causes other issues.

rchl commented 1 year ago

I haven't looked into the code of this package but I guess LSP must be handling such cases already? Maybe should get inspiration from there?

Not sure if relevant but LSP checks view.is_primary() in some places.

jfcherng commented 1 year ago

I haven't looked into the code of this package but I guess LSP must be handling such cases already? Maybe should get inspiration from there?

Yeah. LSP seems working fine. Will check later.

gregsadetsky commented 1 year ago

Just confirmed, it works! Thank you so much for fixing this!!