Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Float Pane #5576

Draft
wants to merge 34 commits into
base: main
Choose a base branch
from
Draft

Float Pane #5576

wants to merge 34 commits into from

Conversation

e82eric
Copy link

@e82eric e82eric commented Jun 17, 2024

Hi, I wanted to make an attempt at #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 e82eric force-pushed the float-pane branch 3 times, most recently from 9bd3c95 to 03eabc7 Compare June 23, 2024 16:38
@derekthecool
Copy link
Contributor

Nice work! Demo video looks good.

@mathjiajia
Copy link

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

@e82eric
Copy link
Author

e82eric commented Jul 3, 2024

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
Copy link
Author

e82eric commented Jul 6, 2024

I think this is ready to be reviewed now.

@Pajn
Copy link

Pajn commented Jul 13, 2024

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.

@e82eric
Copy link
Author

e82eric commented Jul 14, 2024

@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

@Pajn
Copy link

Pajn commented Jul 14, 2024

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.

@Suri312006
Copy link

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

@quantonganh
Copy link
Contributor

@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,

@e82eric
Copy link
Author

e82eric commented Sep 27, 2024

@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?

@quantonganh
Copy link
Contributor

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.

@e82eric
Copy link
Author

e82eric commented Sep 29, 2024

@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

@aleksandersumowski
Copy link

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

@quantonganh
Copy link
Contributor

@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?

@e82eric
Copy link
Author

e82eric commented Oct 5, 2024

@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

@quantonganh
Copy link
Contributor

@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.

@e82eric
Copy link
Author

e82eric commented Oct 5, 2024

@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?

Note if the floating pane is hidden it won't show if the terminal window isn't
focused (it will aprear when the terminal window gets focus), I think this matches the behavior of splits but is more noticeable with the floating pane becoming visible.
@e82eric
Copy link
Author

e82eric commented Oct 5, 2024

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

@quantonganh
Copy link
Contributor

quantonganh commented Oct 5, 2024

@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).

@e82eric
Copy link
Author

e82eric commented Oct 5, 2024

@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.

@quantonganh
Copy link
Contributor

quantonganh commented Oct 6, 2024

@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.

@e82eric
Copy link
Author

e82eric commented Oct 6, 2024

@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?

@quantonganh
Copy link
Contributor

@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"
  }
]

@e82eric
Copy link
Author

e82eric commented Oct 6, 2024

@quantonganh just pushed a fix.

@quantonganh
Copy link
Contributor

@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).

@e82eric
Copy link
Author

e82eric commented Oct 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants