wezterm icon indicating copy to clipboard operation
wezterm copied to clipboard

Float Pane

Open e82eric opened this issue 1 year ago • 39 comments

Hi, I wanted to make an attempt at https://github.com/wez/wezterm/issues/270

This probably needs quite a bit of iteration (this is my first attempt at rust), but wanted to create the pull request to see if this is something you would be open to adding and to see if I am on the right track.

float_pane

e82eric avatar Jun 17 '24 08:06 e82eric

Nice work! Demo video looks good.

derekthecool avatar Jun 29 '24 02:06 derekthecool

it would be nice to have a border for this floating pane

mathjiajia avatar Jul 01 '24 03:07 mathjiajia

Updated with border.

float_pane_with_border2

Configs:

config.float_pane_padding = {
  left = '10%',
  right = '10%',
  top = '10%',
  bottom = '10%',
}
config.float_pane_border = {
  left_width = '0.5cell',
  right_width = '0.5cell',
  bottom_height = '0.25cell',
  top_height = '0.25cell',
  left_color = '#665c54',
  right_color = '#665c54',
  bottom_color = '#665c54',
  top_color = '#665c54',
}

e82eric avatar Jul 03 '24 04:07 e82eric

I think this is ready to be reviewed now.

e82eric avatar Jul 06 '24 00:07 e82eric

Thanks a lot, this is the one feature I've been missing since transition from tmux to wezterm mux 🙏🏼

Tested this and it works really well, just commenting with some small issues I've seen:

  • The float panes all open in my home directory instead of the current directory like split panes and new tabs do
  • The pane selection UI is hard to read if having no split panes and one floating since the underlying pane and the float panes letter overlaps
  • If moving all non floating panes to a new tab or window the floating pane is shown on an otherwise invisible window, would be nice to either disallow this or de-float the floating pane

Screenshot from 2024-07-13 10-26-29 Screenshot from 2024-07-13 10-26-51

A very nice action to have but definitely not required for this to be super useful on its own, is to be able to toggle the floating status of a pane so you can transition a floating pane to a split pane and vice verse.

Pajn avatar Jul 13 '24 08:07 Pajn

@Pajn I think I have the working directory issue fixed now.

For the pane selection my intention was to treat the float like a overlay and turn any selection operations into no-ops and prevent the mouse from selecting a pane (I think there were some scenarios that I missed like the selection UI). I pushed an update that I think turn those into nops now. This may not be the right strategy, is there a scenario where you would want to be able to select/interact with the panes underneath the float?

I think I have it updated so that closing the pane underneath the float is blocked now. Which I think will prevent the scenario where only the float is showing.

I tried out being able to move the float to a split today. It seemed to work, I pushed it to this branch if you want to give it a try. I didn't want to include it in this pull request to keep the scope down. https://github.com/e82eric/wezterm/tree/move-float-to-split

This is what it looked like. move_float_to_split

e82eric avatar Jul 14 '24 07:07 e82eric

Wow! I was just starting to familiarize myself with the code to see if I could help but not at that speed :) Very quick testing have all the issues I experienced fixed.

This may not be the right strategy, is there a scenario where you would want to be able to select/interact with the panes underneath the float?

I was mostly just trying to break it to see where the limitations where. The one possible usecase I can see is to move one background pane to another tab/window if you need access to it after realizing the float pane was longer lived than you intended. However I think that's better solved by being able to transition the float pane to a split anyway. And disabling those actions when in a float definitely avoids a whole class of potential problems.

I didn't want to include it in this pull request to keep the scope down.

Totes! I was just excited. Thanks a lot for the branch though, will definitely start playing with it. And if there is anything I can do to help get this merged, let me know.

Pajn avatar Jul 14 '24 07:07 Pajn

i really like the work man, good stuff been wanting this feature, as im coming from zellij that has a really nice implementation of floating panes

Suri312006 avatar Jul 14 '24 17:07 Suri312006

Thanks! updated

e82eric avatar Aug 09 '24 01:08 e82eric

What is the status of this PR? Really great feature&work btw!

dev-lop36 avatar Sep 17 '24 15:09 dev-lop36

Thanks! I think this is more or less still pending review. I am happy to put in the work to get this across the the finish line, but don't want bug Wez about it, I am sure he has a million priorities. In the mean time if anyone wants to give it a try and notice any issues or want to review the code, I can work through those.

e82eric avatar Sep 18 '24 04:09 e82eric

Thanks Wez! I pushed a commit to address the initial feedback.

I think the multiplexing, scrollbar and lua events are probably all gaps. Will start working through those and will @ you when I get through them. Marking the PR as draft until I get through them.

e82eric avatar Sep 22 '24 00:09 e82eric

@e82eric Thank you for taking your time to implement this.

For the pane selection my intention was to treat the float like a overlay and turn any selection operations into no-ops and prevent the mouse from selecting a pane (I think there were some scenarios that I missed like the selection UI). I pushed an update that I think turn those into nops now. This may not be the right strategy, is there a scenario where you would want to be able to select/interact with the panes underneath the float?

I'm using Helix as my IDE with the help of WezTerm and other CLI tools (lazygit, tig, fzf, gh, ...). For example, I've set up key bindings to run my code in the bottom pane, or connect to a database in the right pane using lazysql, ... Actually, I've been trying your PR to move some of these actions to a floating pane. However, I find myself needing to frequently switch between the floating pane and the one beneath it. It would be really helpful if we can do that by using some shortcut keys.

Thanks,

quantonganh avatar Sep 27 '24 07:09 quantonganh

@quantonganh right now it has a default of CTRL|ALT|SHIFT(P) for creating the float pane, and can be closed using the normal binding for CloseCurrentPane. It can be updated in the config with something like this.

config.keys = {
    {
      key = 'e',
      mods = 'CTRL|SHIFT',
      action = wezterm.action.FloatPane,
    },
    key = 'w',
      mods = 'CTRL|SHIFT',
      action = wezterm.action.CloseCurrentPane { confirm = false },
    }
}

Does that help or am I misunderstanding the problem? Are you looking for something like being able to hide the floating pane instead of closing it when switching between the floating pane and the one beneath it?

e82eric avatar Sep 27 '24 23:09 e82eric

Are you looking for something like being able to hide the floating pane instead of closing it when switching between the floating pane and the one beneath it?

Exactly! For example: while coding, I might want to check something in the database, I can bind a key to send lazysql to the floating pane. Afterward, I want to switch back to my main pane (Helix editor) without closing the floating pane - just hiding it.

I realized that to achieve this, we might need to add additional values, such as floating and beneath to the direction argument for the get-pane-direction command. This would allow checking whether a floating pane already exists.

quantonganh avatar Sep 28 '24 06:09 quantonganh

@quantonganh if you get a chance can you try the latest commit. I made an attempt at it and wanted to see if you notice anything weird and if it matches what you were looking for, I think being able to hide the float may make more sense ux wise (I can now just hide the floating pane if someone clicks outside of it, etc).

ctrl+shift+e is the default for ToggleFloatingPane

e82eric avatar Sep 29 '24 05:09 e82eric

Do you expect to be able to have more than one floating pane at a time? My use case would be to have short lived panes for interactive use, eg to implement extracto like functionality. I was wondering if long lived pane and short lived ones could co-exist

aleksandersumowski avatar Sep 29 '24 07:09 aleksandersumowski

@e82eric It's working fine now and I can use Ctrl+Shift+E to toggle the floating pane. Thank you!

Regarding the wezterm cli list --format json, I think it would be useful to add a field like is_floating, similar to is_active and is_zoomed, to indicate whether a pane is floating.

My workflow would be something like this:

  • Check if there's a floating pane in the current tab
  • If there is, send the command directly to the floating pane
  • If not, spawn a floating pane and send the command to it

To support this, we'd also need to enable wezterm cli activate-pane to work with floating panes.

What do you think?

quantonganh avatar Sep 29 '24 10:09 quantonganh

@quantonganh have been trying to get multiple floating panes working and it has taken a lot longer than expected.

For the cli list, it is probably going to take some iteration to get the data model right.

The float floating panes are in their own storage outside of the regular panes which are a binary tree and the floating panes are a regular list. Would it work in your use case if cli list just had the clients in the list but not an indication of it is a floating pane or not and then separate command for list-floating-panes? Was thinking it would be useful to have a sperate command with floating pane specific information like what index the pane is so you toggle floating panes by index ex leader 1 toggles lazygit and leader 2 toggles htop

e82eric avatar Oct 05 '24 02:10 e82eric

@e82eric I don't currently need multiple floating panes, but I do find the feature useful. That was the question from @aleksandersumowski.

Would it work in your use case if cli list just had the clients in the list but not an indication of it is a floating pane or not and then separate command for list-floating-panes?

In my case, that's fine. But I'm not sure if we should have a separate command for listing floating panes. @wez might have more thoughts on this.

quantonganh avatar Oct 05 '24 02:10 quantonganh

@quantonganh That makes sense. I will try to add the is_floating property and see why activate-pane isn't working.

It looks like Zellij will create the floating pane if one doesn't exist when you call toggle-floating-pane, if this did the same would it simplify the workflow at all?

e82eric avatar Oct 05 '24 04:10 e82eric

@quantonganh I just pushed the changes for the is_floating property and active-pane. If you get a chance could you try it out?

e82eric avatar Oct 05 '24 05:10 e82eric

@e82eric I've pulled your latest commits and tested it, but there seems to be an issue with listing panes:

$ ./target/release/wezterm cli list --format json
14:56:30.039  ERROR  wezterm_client::client > Error while decoding response pdu: Invalid Bool Encoding
14:56:30.039  ERROR  wezterm                > Error while decoding response pdu: Invalid Bool Encoding; terminating

Additionally, I'm unable to switch between the normal and floating panes using activate-pane (though switching between normal panes works fine).

quantonganh avatar Oct 05 '24 08:10 quantonganh

@quantonganh can you try to restart wezterm-mux-server, think that error happens when the mux server is still running on the previous version that doesn't know to send the new is_floating property.

e82eric avatar Oct 05 '24 16:10 e82eric

@e82eric The issue still persists when starting wezterm-mux-server first (as a separate processs):

$ ps -ef | grep wezterm
  501 26436     1   0 10:45AM ??         0:00.84 ./e82eric/wezterm/target/release/wezterm-mux-server --pid-file-fd 3
  501 46561 83615   0 11:00AM ttys000    0:01.45 ./e82eric/wezterm/target/release/wezterm-gui connect unix
$ ./target/release/wezterm cli list --format json
11:02:12.487  ERROR  wezterm_client::client > Error while decoding response pdu: Invalid Bool Encoding
11:02:12.488  ERROR  wezterm                > Error while decoding response pdu: Invalid Bool Encoding; terminating

Please note that, in some cases, the user might start wezterm-gui directly without first launching wezterm-mux-server.

quantonganh avatar Oct 06 '24 04:10 quantonganh

@quantonganh I bumped the codec version, can you try a clean and build, make sure there aren't any running wez* processes and try again?

e82eric avatar Oct 06 '24 05:10 e82eric

@e82eric Thank you!

The listing of panes and switching between the floating pane and the pane beneath it are working as expected. However, it seems the is_active field of the floating pane does not update when switching back to the normal pane:

$ echo $WEZTERM_PANE
0

$ ./target/release/wezterm cli list --format json
[
  {
    "window_id": 0,
    "tab_id": 0,
    "pane_id": 0,
    "window_title": "./target/release/wez ~/C/g/e/wezterm",
    "is_active": true,
    "is_zoomed": false,
    "is_floating": false,
    "tty_name": "/dev/ttys001"
  },
  {
    "window_id": 0,
    "tab_id": 0,
    "pane_id": 2,
    "window_title": "./target/release/wez ~/C/g/e/wezterm",
    "is_active": true,
    "is_zoomed": false,
    "is_floating": true,
    "tty_name": "/dev/ttys002"
  }
]

quantonganh avatar Oct 06 '24 07:10 quantonganh

@quantonganh just pushed a fix.

e82eric avatar Oct 06 '24 07:10 e82eric

@e82eric It's ok now:

$ echo $WEZTERM_PANE
0

$ ./target/release/wezterm cli list --format json
[
  {
    "window_id": 0,
    "tab_id": 0,
    "pane_id": 0,
    "window_title": "./target/release/wez ~/C/g/e/wezterm",
    "is_active": true,
    "is_zoomed": false,
    "is_floating": false,
    "tty_name": "/dev/ttys001"
  },
  {
    "window_id": 0,
    "tab_id": 0,
    "pane_id": 2,
    "window_title": "./target/release/wez ~/C/g/e/wezterm",
    "is_active": false,
    "is_zoomed": false,
    "is_floating": true,
    "tty_name": "/dev/ttys002"
  }
]

However, for some reasons, wezterm cli activate-pane --pane-id 0 does not switch back from the floating pane to the normal pane, although it works perfectly in the reverse direction (from the normal pane to the floating pane).

quantonganh avatar Oct 06 '24 08:10 quantonganh

@quantonganh just pushed fix for activate-pane --pane-id 0

e82eric avatar Oct 06 '24 08:10 e82eric