sublimelsp / LSP-julia

Julia support for Sublime's LSP plugin using LanguageServer.jl
MIT License
23 stars 1 forks source link

add open REPL in tab option #16

Closed ghyatzo closed 2 years ago

ghyatzo commented 2 years ago

Instead of only giving the option of opening up the REPL in the panel view, this commit adds a command julia_open_repl_tab for opening a terminus window with a julia session in a tab view instead.

Adds the command for the command palette entry and differentiate between the panel and tab view commands

swap back focus after code send

ghyatzo commented 2 years ago

addresses #14

jwortmann commented 2 years ago

Thanks for the contribution!

Instead of using two separate commands, can we combine the functionality into the existing julia_open_repl command, and specify the panel/tab choice as an argument to the run method? That should even be backwards compatible, in case someone did set up a keybinding for the command.

Like this:

{
    "caption": "LSP-julia: Open Julia REPL in Panel",
    "command": "julia_open_repl",
    "args": {"panel": true}
},
{
    "caption": "LSP-julia: Open Julia REPL in Tab",
    "command": "julia_open_repl",
    "args": {"panel": false}
},
class JuliaOpenReplCommand(sublime_plugin.WindowCommand):

    def run(self, panel: bool = True) -> None:
        if panel:
           # ...
        else:
           # ...
ghyatzo commented 2 years ago

Yes, ill merge the two functions.

ghyatzo commented 2 years ago

Implemented the changes that you asked. Had some doubts about the conflicting scenarios where you would first open a panel , forgot about it and then opened a tab (and vice versa). Opted to just passive aggressively bring the already existing one into focus no matter the command you select.

Wanted to confront with you before making more drastic behaviour logic.

jwortmann commented 2 years ago

Had some doubts about the conflicting scenarios where you would first open a panel , forgot about it and then opened a tab (and vice versa). Opted to just passive aggressively bring the already existing one into focus no matter the command you select.

Yes, we could alternatively run the "Terminus: Maximise to Tab" or "Terminus: Minimize to Panel" commands in those cases. Not sure if that would be better, but I'm fine with the current solution to just focus the existing one, as long as it is ensured that only one REPL can exist at a given time.

But I see a problem when you run "Terminus: Minimize to Panel" manually. The panel won't have the correct "panel_name" attribute in that case, and everytime you send another code block, it will create a new panel with name "Output: Terminus" (instead of "Output: Julia REPL") then. I'm not sure if this is a bug in Terminus, or if we can fix this problem on our side somehow. I would expect that Terminus for example assigns the tab name as "panel_name" or so, when the "Terminus: Minimize to Panel" command is run. I think the panel still has the correct "tag" attribute, and that will mess with the logic in Terminus or here in this plugin where we try to create a new panel, which doesn't work then because there already exists one with the same "tag".

ghyatzo commented 2 years ago

Yes, initially i tried using the terminus minimise functions but encountered the same issue of disconnecting the terminal reference. That is what started this PR basically.

I don't really know the internals of terminus, but I am not so sure there is a quick and easy solution we can implement from our side.

At least the maximise to tab works just fine

ghyatzo commented 2 years ago

I lurked a bit in their source code, when the minimise command is issued, the terminal session is detached, the view is closed, a new panel gets opened, all previous text copied back in and the session is reattached to the new one.

In the reinitialisation there is the line panel_name = view.settings().get("terminus_view.panel_name", None)

and it seems to be only read in that instance, and not when creating a new terminal. So maybe we could set that specific setting.

jwortmann commented 2 years ago

I guess the setting is not preserverd on another minimize/maximize, because a new view will be created each time. It doesn't seem to be a good solution to listen for the corresponding Terminus commands and adjust the settings everytime... I thought we could maybe use

    window.run_command("terminus_open", {
        "cmd": cmd,
        "cwd": "${file_path:${folder}}",
        "title": JULIA_REPL_NAME,
        "panel_name": JULIA_REPL_NAME,
        "show_in_panel": panel,
        "focus": focus,
        "tag": JULIA_REPL_TAG,
        "env": settings.get("repl_env_variables"),
    })

in start_julia_repl() to ensure that the panel_name will always be set, but Terminus seems to always create it in a panel then and it will still lose the panel_name on minimize. I see there was a recent commit https://github.com/randy3k/Terminus/commit/0aadf611f25f48dafec47da88893621ee197b28f with a related change, but no new version with that change published yet. I don't know if this maybe will allow us to use the above snippet. But I think Terminus should use the tab title as panel_name if it isn't set, when minimizing to panel. I will create an issue for it in the Terminus repo and see what Randy says about it -> https://github.com/randy3k/Terminus/issues/331

ghyatzo commented 2 years ago

I tried playing a bit with that idea, and it works. I tried ensuring that the tab view always has that setting. but other than being quite janky, it still fails if you maximise and then minimise without interacting with the terminus window through LSP-julia in some way. At that point the setting does not get refreshed and upon minimisation it breaks.

EDIT: We reached the same conclusion, nice. Missed your message before posting my own. Yes, my approach was very very messy to begin with. hopefully randy gives the green, what you proposed seems a very reasonable behaviour. Ill wait for updates.

jwortmann commented 2 years ago

If I understand it correctly, having the panel_name attribute always set will work with the next Terminus release. Minimizing and maximizing should work without problems then. So I would suggest that we wait for the next Terminus release, make the little adjustment to panel_name and show_in_panel like in the code snippet above, and then I can merge this PR.

jwortmann commented 2 years ago

There was finally a new release of Terminus. I made some small adjustments, and it looks like the maximize/minimize from tab to panel works fine now. And I think the focus_view(edit_view) was not needed. At least when I tried it, the edited file already keeps the focus, so the Terminus command seems to already handle that.

I will try to update the language server to the latest version and make a new release soon.