MLFlexer / resurrect.wezterm

A plugin to restore windows, tabs and panes for Wezterm inspired by tmux-resurrect
MIT License
74 stars 2 forks source link

fix bug with named ssh_domain not being saved #45

Closed MLFlexer closed 2 months ago

MLFlexer commented 2 months ago

This PR fixes a bug with ssh_domains not being saved, it also simplifies saving all kinds of domains.

MLFlexer commented 2 months ago

This PR removes the checking of substrings within the domain name. This also means that the cwd is not being set for wsl.

It seems like wsl has problems with setting the cwd, as it is also a problem when spawning a new tab or splitting a pane from an already existing wsl pane. The expected behaviour is that the cwd of the existing pane would be used for the new panes, but this is not the case from my testing.

As such it seems to be an upstream issue, either in wezterm or wsl. Thus i am conflicted weather to include setting the cwd for wsl based on the title or not, as the title could be changed by the user and thus it would not work.

MLFlexer commented 2 months ago

I havn't tested it fully yet, but this PR in its current state might close #40, #43 and #44

joncrangle commented 2 months ago

I just tested this PR successfully with an SSH and docker domain in their own tabs and was able to resurrect them.

I encountered a minor bug. If tab 1 was a docker domain and tab 2 was local, there was an issue spawning tab 2 as it still thought it was part of the docker domain. I was able to resolve by always saving root.domain. This also eliminates the else block that follows if domain == "local" then:

    local domain = root.pane:get_domain_name()
    if not wezterm.mux.get_domain(domain):is_spawnable() then
        wezterm.log_warn("Domain " .. domain .. " is not spawnable")
        wezterm.emit("resurrect.error", "Domain " .. domain .. " is not spawnable")
    else
        root.domain = domain
        if not root.pane:get_current_working_dir() then
            root.cwd = ""
        else
            root.cwd = root.pane:get_current_working_dir().file_path
            if is_windows then
                root.cwd = root.cwd:gsub("^/([a-zA-Z]):", "%1:")
            end
        end

        if domain == "local" then
            -- pane:inject_output() is unavailable for non-local domains,
            -- not saving scrollback because it would slow down the process
            -- See: https://github.com/MLFlexer/resurrect.wezterm/issues/41
            root.process = root.pane:get_foreground_process_name()
            if pub.get_shell_process(root.process) then
                local nlines = root.pane:get_dimensions().scrollback_rows
                if nlines > pub.max_nlines then
                    nlines = pub.max_nlines
                end
                root.text = root.pane:get_lines_as_escapes(nlines)
            end
        end
    end

I think this results in the following README updates:

## Features
...
* Re-attach to remote domains (e.g. SSH, SSHMUX, WSL, docker, etc.).
...

and

               "pane_tree":{
                  "cwd":"/home/user/",
                  "domain": "local",
                  "height":50,
                  "index":0,
                  "is_active":true,
                  "is_zoomed":false,
                  "left":0,
                  "pixel_height":1000,
                  "pixel_width":1910,
                  "process":"/bin/bash", -- value is empty if attached to a remote domain
                  "text":"Some text", -- not saved if attached to a remote domain, see https://github.com/MLFlexer/resurrect.wezterm/issues/41
                  "top":0,
                  "width":191
               },
MLFlexer commented 2 months ago

Thanks for testing! Added the changes you suggested :smile:

MLFlexer commented 2 months ago

Just tested with ExecDomains and it seems to work fine with connecting to docker domains like in the example from the docs

MLFlexer commented 2 months ago

This PR removes the checking of substrings within the domain name. This also means that the cwd is not being set for wsl.

It seems like wsl has problems with setting the cwd, as it is also a problem when spawning a new tab or splitting a pane from an already existing wsl pane. The expected behaviour is that the cwd of the existing pane would be used for the new panes, but this is not the case from my testing.

As such it seems to be an upstream issue, either in wezterm or wsl. Thus i am conflicted weather to include setting the cwd for wsl based on the title or not, as the title could be changed by the user and thus it would not work.

I get the correct cwd on my wsl and ssh sessions if the remote shell i am connecting to has correct shell integration. So If you are experiencing problems with cwd @joncrangle then I would advice you to use the (wezterm.sh)[https://github.com/wez/wezterm/blob/main/assets/shell-integration/wezterm.sh] script on your remote shells