-
-
Notifications
You must be signed in to change notification settings - Fork 964
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
handle CSI resize events #3785
handle CSI resize events #3785
Conversation
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
This might be a little rough, but it works. I welcome any feedback you have and I'll make improvements as suggested. I feel like I'm missing one minor operation in the low level resize handling code. When I issue a resize, the OS window resizes as expected, but if I click lower-right as if to resize it snaps back to the original size. This can be tested with:
One minor flaw is that when a tab-bar is present it ends up resizing to 1 character shorter than requested due to the tab-bar. By this I mean I see the following behavior:
|
I'm still investigating this, but it is even worse than I wrote here. When the window loses focus the |
On Wayland IIRC programs are only allowed to resize their windows during |
There was a missing call to
Agree it will not do anything useful for tiling window managers. It is quite useful for non-tiling window managers as there is currently no other way to resize the OS window. (
I've tested this on X11, Wayland, and MacOS now. It works on all 3 except on MacOS it ends up with I've repushed my latest changes to https://github.com/williamspatrick/kitty/tree/csi-resize , which was where this PR was from. I think if you re-open the PR it will refresh from that branch as well. |
Well, I am happy that you got it to work on Wayland, am surprised it does, but good. However, I dont think the CSI escape code is really suitable. Imagine a kitty OS Window with multiple windows inside. If a CSI request to resize one of those windows is received, should the multi-window layout be resized or the OS Window or both/neither? Incidentally which wayland window manager are you testing it with? If it does indeed work on wayland, I am willing to consider adding os window resize functionality, but I think rather than the CSI code, a better fit would be remote control, where you can add a kitty @ resize-os-window command alongside the existing resize-window command. And github doesnt let me reopen this PR so I suggest we settle ona design here and then you can opena new one. |
Sure thing.
Gnome 40 / mutter.
Right. I recognized this condition and currently detected it.
What I chose to do right now is drop the CSI request if there are multiple windows. (The I think the ideal implementation would be to do some layout calculations so that the sub-window is resized to the CSI amount and the other windows are reflowed per the current layout. But, for the time being I left it as 'neither' until someone enterprising enough to tackle the layout part of it comes around. gnome-terminal and xterm both support the CSI sequence. If you run something like gnome-terminal+tmux, tmux eats the CSI. To me, 'neither' would mimic the behavior someone might expect coming from that. It would be trivial to change this to 'OS window' + reflow, but from a user perspective that seems a little surprising so I didn't do it that way. Deleting the above if-conditions would get us to this behavior if you think it is better though.
I can certainly add a I see a few obvious options:
I think that 2+3 is something I could implement in rather short order if that is acceptable to you. |
On Fri, Jul 02, 2021 at 05:22:35AM -0700, Patrick Williams wrote:
Gnome 40 / mutter.
Would be nice to check if resizing actually works on at least one other
window manager.
> Imagine a kitty OS Window with multiple windows inside. If a CSI request to resize one of those windows is received, should the multi-window layout be resized or the OS Window or both/neither?
Right. I recognized this condition and currently detected it.
```
+ if len(self.windows) != 1:
+ log_error('Unable to resize layout-ed windows.')
+ return
+
+ if window is not self.active_window:
+ log_error('Unable to resize inactive window.')
+ return
```
What I chose to do right now is drop the CSI request if there are multiple windows. (The `if is not active_window` might be redundant?)
I think the ideal implementation would be to do some layout calculations so that the sub-window is resized to the CSI amount and the other windows are reflowed per the current layout. But, for the time being I left it as 'neither' until someone enterprising enough to tackle the layout part of it comes around.
You can just piggy back of the existing resize-window command, see
rc/resize_window.py
gnome-terminal and xterm both support the CSI sequence. If you run something like gnome-terminal+tmux, tmux eats the CSI. To me, 'neither' would mimic the behavior someone might expect coming from that.
It would be trivial to change this to 'OS window' + reflow, but from a user perspective that seems a little surprising so I didn't do it that way. Deleting the above if-conditions would get us to this behavior if you think it is better though.
> I think rather than the CSI code, a better fit would be remote control, where you can add a kitty @ resize-os-window command alongside the existing resize-window command.
I can certainly add a `resize-os-window` as well but, as a user, I really like the CSI codes as well. I spend a fair amount of time connected to remote machines for development and have my dotfiles synchronized between them. It is nice to be able to run something like `resize large && htop && resize default` and have it do the expected thing no mater which machine I'm on. Without the CSI support I wouldn't be able to resize my local window from a remote server.
remote control works over ssh as well. Though there is the caveat that
you need to trust the machines you are SSHing into as remote control
over ssh means any program you run on the remote machine can control
kitty.
I see a few obvious options:
1. Keep CSI support as is here.
2. Add an option to enable this CSI support.
3. Add a `kitty @ resize-os-window`.
If you do want to use CSI then I think this should be done "right".
Right for me means:
1) Implement support for CSI 8 9 and 10 t i.e. resize, maximize and
fullscreen
2) CSI 8 t should be "smart":
- if there is a single window in the current layout, resize the OS
Window
- if there are multiple windows, resize the OS Window enough so that
the reflowed layout satisfies the resize request as much as
possible, and further resize the layout to try to get closer to the
requested size
3) Add CSI 88 t and CSI 89 t that resize only OS Window and only window
in layout.
4) Add a resize-os-window remote control command
5) Add an option to turn off window resizing via CSI codes. Report the
status of this option via
https://sw.kovidgoyal.net/kitty/kittens/query_terminal.html (see how it
is done for the allow_hyperlinks option).
I realize this may be more work than you are willing to do, so I am also
ok with just a remote control command and no CSI support. If you do
decide to do it, I can help you with the resizing the layouts part.
|
I'll give something else a try as well. Do you have any suggestions that would be best for more of your users?
Thanks for the pointer.
tbh, I've only been using kitty for a few days. Doesn't the remote control over ssh also require a kitty binary to be put on the remote machine? I think that's the bigger hang-up for me.
Will do. 9/10 should be trivial. For reference (https://ttssh2.osdn.jp/manual/4/en/about/ctrlseq.html):
Understood. The single window part is done now. I'll see about the reflow part using the code pointer you gave me above.
Do you have any reference on 88/89 or did you make these numbers up? I'm not seeing any sources. I would assume these should follow the format of
I'm planning to focus on CSI first. Will let you know how much progress I make.
Yep.
I think most of these are fairly doable. |
On Fri, Jul 02, 2021 at 06:49:51AM -0700, Patrick Williams wrote:
> Would be nice to check if resizing actually works on at least one other window manager.
I'll give something else a try as well. Do you have any suggestions that would be best for more of your users?
weston and/or kwin would be my choices.
> remote control works over ssh as well. Though there is the caveat that you need to trust the machines you are SSHing into as remote control over ssh means any program you run on the remote machine can control kitty.
tbh, I've only been using kitty for a few days. Doesn't the remote control over ssh also require a kitty binary to be put on the remote machine? I think that's the bigger hang-up for me.
No, it doesn't the remote control protocol is a simple one that can even
be used from shell scripts, see: https://sw.kovidgoyal.net/kitty/rc_protocol.html
> 3) Add CSI 88 t and CSI 89 t that resize only OS Window and only window in layout.
Do you have any reference on 88/89 or did you make these numbers up? I'm not seeing any sources.
I would assume these should follow the format of `8`.
I made them up, they are unused so far as I know, the final numbers can
always be decided later. And yes same format, aka a width and height in
cells, with 0 being full screen.
|
I launched a nested Weston instance and resize works the same as I'm seeing on Mutter. |
No description provided.