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

Change the enabled layouts via remote control #4129

Closed
page-down opened this issue Oct 17, 2021 · 24 comments
Closed

Change the enabled layouts via remote control #4129

page-down opened this issue Oct 17, 2021 · 24 comments

Comments

@page-down
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently the enabled layout can be configured in the configuration file and startup session, however there are times when the enabled layouts of the current tab needs to be changed to make it easier for ctrl+shift+l to switch between layouts.

Describe the solution you'd like
Add a new command to change enabled layout for remote control. There may be option to change the default layouts (for new tabs), or to change the enabled layouts of the matching tab only.

It also supports binding the keyboard as a complement to the Layout Management feature set.
https://sw.kovidgoyal.net/kitty/conf/#layout-management

Describe alternatives you've considered
n/a

Additional context

This suggestion is mentioned in:
#4110 (comment)

@page-down
Copy link
Contributor Author

page-down commented Oct 18, 2021

Thank you. May I ask would you consider adding it as a mappable action as well? This makes it easier for the user to config, works without the remote_control part.

Here is my patch to the documentation part.

diff --git a/kitty/options/definition.py b/kitty/options/definition.py
index 25753bd9..349bc2a7 100644
--- a/kitty/options/definition.py
+++ b/kitty/options/definition.py
@@ -3241,6 +3241,16 @@
     'next_layout kitty_mod+l next_layout',
     )
 egr('''
+You can switch between different sets of layouts::
+
+    map ctrl+alt+l>1 set_enabled_layouts *
+    map ctrl+alt+l>2 set_enabled_layouts fat,grid,stack
+    map ctrl+alt+l>3 set_enabled_layouts tall,vertical,splits
+
+To setup enabled layouts for all newly created tabs::
+
+    map ctrl+alt+l>a set_enabled_layouts --configured *
+
 You can also create shortcuts to switch to specific layouts::
 
     map ctrl+alt+t goto_layout tall

I found that the layout with parameters does not work.
For example: tall:bias=50;full_size=1;mirrored=false,grid,stack

Since such a powerful layout feature is already available, perhaps it is worth taking full advantage of it.
Is it possible to specify layout parameters in set-enabled-layouts or goto-layout?

This allows users to set up frequently used layouts, and then quickly switch between different parameters and use them. Instead of being limited to a fixed set of layouts in the session file or configuration. (Currently the parameters can only be configured in the configuration file.)

@page-down
Copy link
Contributor Author

https://sw.kovidgoyal.net/kitty/remote-control/#mapping-key-presses-to-remote-control-commands

Oh, that's how I have it configured now. Since we already have direct mappable actions like next_layout, perhaps it would be convenient to provide this mappable action as well, without having to define the remote_control part.

The examples provided in the sample config documentation can also be inspiring for users. This is just my personal opinion. Thank you. :)

@kovidgoyal
Copy link
Owner

IMO this is not a common enough action to warrant a default mapping and
without that, it wont be particularly discoverable anyway.

@page-down
Copy link
Contributor Author

IMO this is not a common enough action to warrant a default mapping and without that, it wont be particularly discoverable anyway.

Yes, it is also logically inappropriate to set default keys, as everyone's needs are different, so it is only suitable to appear in the example document.

Anyway, my point is to make it easier for users to use kitty's powerful layout features, and I respect your decision.

I found an issue while learning about the code base and trying to add it as a mappable action.

kitty --config=NONE -o 'map f1 set_colors --configurd typo-or-invalid-option'

The first parameter above was entered incorrectly, and pressing F1 at this point will cause the main process to exit. It might be possible to catch this exception to avoid the whole software from quitting.

This issue is not related to the topic, but I don't know if I need to create an issue, since I don't want to make too much noise.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Oct 18, 2021 via email

@page-down
Copy link
Contributor Author

There is also an issue with the layout parameters, which I will also describe here.

The following examples are given in the documentation.
https://sw.kovidgoyal.net/kitty/layouts/

enabled_layouts tall:bias=70;full_size=2

This means that the mirrored parameter is optional and uses the default value of false.

Is it possible to use default values for other parameters that are not explicitly defined?
However, with the following configuration, mirrored=true is not working. The new windows are still created on the right side.

kitty --config=NONE -o "enabled_layouts tall:mirrored=true"

@kovidgoyal
Copy link
Owner

You want

kitty --config=NONE -o 'enabled_layouts=tall:mirrored=true'

@page-down
Copy link
Contributor Author

You want
kitty --config=NONE -o 'enabled_layouts=tall:mirrored=true'

Thank you, my bad. Get used to the syntax like -o 'map f1 ...' ...

The root cause of the question mentioned above is the same.

Is it possible to specify layout parameters in set-enabled-layouts or goto-layout?

The answer is yes.

kitty -o 'map=f1 remote_control set-enabled-layouts tall:mirrored=true'

It works fine after using the key=value syntax, or pulling the latest commit. No more kitty.rc.base.RemoteControlError: The window layout true is unknown.

I should have checked every aspect more carefully before posting.


I have a question about the layout-related features during my recent experience with them.

Is it possible to get the enabled layouts in tab?

For example, before using goto-layout, you need to know the exact parameters of the enabled layout. Also, if you use a custom script to change a parameter in the layout and leave the rest unchanged, you need to know about the enabled layouts.

I also found that in kitty @ ls, the parameter information for the activated layout is missing, e.g. mirrored=true.

There can be multiple same layouts with different parameters in enabled layouts, without parameters it is difficult to know which one it is exactly from the data.

This is perhaps too much to ask. It would be great if you could consider it.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Oct 21, 2021 via email

@page-down
Copy link
Contributor Author

Sure done

Thanks. Is there going to be a new get-enabled-layouts command for getting all the layouts that are enabled for matching tabs? I'll try it later.

With the latest commit, the current activated layout parameters are available in kitty @ ls. I'll do another conversion to enabled_layouts syntax.


I found in my recent usage that switching between layout mirrored=true and mirrored=false (ctrl+shift+l), there is a serious delay.

kitty --config=NONE -o enabled_layouts=tall:mirrored=false,tall:mirrored=true

Open three kitty windows after startup. Press ctrl+shift+l to toggle the layout. There is a noticeable delay when switching layouts. The same problem with fat:mirrored=true/false.

However, switching between different layouts like tall:mirrored=false,fat:mirrored=true,grid and so on, there is no any delay.

I have only found this problem under macOS and have not been able to reproduce it under Linux.

I tried make profile. However, after kitty exits, there are a lot of error messages. The final profile log file /tmp/kitty-profile.log seems to have no function names in it.

error: .../otool-classic: can't open file: /usr/lib/.../***.dylib  (No such file or directory)
error: .../otool-classic: can't open file: /System/Library/Frameworks/.../***.dylib  (No such file or directory)
Total: 103 samples
      35  34.0%  34.0%       35  34.0% 0x00007fff2037d2ba
      18  17.5%  51.5%       18  17.5% 0x00007fff2037d3f2
       6   5.8%  57.3%        6   5.8% 0x00007fff2037fcde
       5   4.9%  62.1%        5   4.9% 0x00007fff2037d25a
...

Do you have any thoughts on the delays here? Thank you.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Oct 21, 2021 via email

@page-down
Copy link
Contributor Author

What delay? I can see about a second of delay while the shell responds to the window being resized and redraws the prompt. But that's it.

It's that lag. Compare with tall:mirrored=false,fat:mirrored=true,grid,vertical or switch between any other layout.
Any other switch between layouts is done instantly. For example, with 3 empty windows. map f1 launch read

@kovidgoyal
Copy link
Owner

I see the same delay with tall and vertical. Note that the delay doesn't
occur every time, and the freq is a bit less with tall -> vertical

@page-down
Copy link
Contributor Author

Is it enabled_layouts=tall,vertical? I haven't tried this combination before, and I just tried it many times, and on my system, this combination has zero latency. That' s strange.

However, the phenomenon you describe is exactly what I encountered, sometimes lagging, and then fast when you switch back.

@kovidgoyal
Copy link
Owner

This diff will tell you exactly the timing of the layout being switched
and the subsequent signal being sent. As long as those are quick, kitty
is doing its job and the latency is elsewhere:

diff --git a/kitty/tabs.py b/kitty/tabs.py
index cbef1aed..2507a276 100644
--- a/kitty/tabs.py
+++ b/kitty/tabs.py
@@ -253,6 +253,7 @@ def next_layout(self) -> None:
             else:
                 idx = -1
             nl = self.enabled_layouts[(idx + 1) % len(self.enabled_layouts)]
+            log_error(f'switching layout to {nl}')
             self._set_current_layout(nl)
             self.relayout()

diff --git a/kitty/window.py b/kitty/window.py
index 4e256a67..7143af40 100644
--- a/kitty/window.py
+++ b/kitty/window.py
@@ -561,6 +561,7 @@ def set_geometry(self, new_geometry: WindowGeometry) -> None:
             self.screen.lines, self.screen.columns,
             max(0, new_geometry.right - new_geometry.left), max(0, new_geometry.bottom - new_geometry.top))
         if current_pty_size != self.last_reported_pty_size:
+            log_error('sending SIGWINCH')
             get_boss().child_monitor.resize_pty(self.id, *current_pty_size)
             if not self.pty_resized_once:
                 self.pty_resized_once = True

@page-down
Copy link
Contributor Author

This diff will tell you exactly the timing of the layout being switched and the subsequent signal being sent. As long as those are quick, kitty is doing its job and the latency is elsewhere:

Yes, I can see that debug log was printed immediately after executing the next layout action.

I also found that if you press a key (e.g. any letter key) immediately after performing next_layout, the latest layout is immediately rendered and the pressed letter appears in the shell.

I have yet to find out what is causing the 1 second delay in rendering that occurs when switching between specific layouts. There is no problem with the log output, keyboard input response.

@kovidgoyal
Copy link
Owner

you can try building with make debug-event-loop and see if rendering is
delayed after the resize for some reason.

@kovidgoyal
Copy link
Owner

And I should note for me it does not reproduce at all while running locally, only while running under VNC, so as far as I can tell this is a bug in a layer under kitty.

@page-down
Copy link
Contributor Author

you can try building with make debug-event-loop and see if rendering is delayed after the resize for some reason.

I finally found the source of this 1 second time interval. After using escape code to turn off cursor blinking, it doesn't re-render at all after switching layouts, no matter how long you wait.

The following is the normal switching between layouts. Even if the cursor is not blinking, it will be re-rendered immediately.

switching layout to vertical
sending SIGWINCH
sending SIGWINCH
sending SIGWINCH
[20.3997] Processing global state
[20.3997] input_read: 0, check_for_active_animated_images: 0
[20.4027] Processing global state
[20.4027] input_read: 0, check_for_active_animated_images: 0
[20.4099] State check timer fired

After stopping the cursor blinking, for some layouts, no matter how many times ctrl+shift+l is pressed, it will not re-render. There is only this log switching layout to ... (with some keyboard event log).

switching layout to tall:mirrored=false
[14.7206] Processing global state
[14.7206] input_read: 0, check_for_active_animated_images: 0
[14.7783] Processing global state
[14.7783] input_read: 0, check_for_active_animated_images: 0
[14.9932] Processing global state
[14.9932] input_read: 0, check_for_active_animated_images: 0
[14.9974] Processing global state
[14.9974] input_read: 0, check_for_active_animated_images: 0
[15.0032] State check timer fired

@kovidgoyal
Copy link
Owner

doesnt repro for me, and that log indicates sending SIHWINCH is not
happening, look in the set_geometry function in window.py to see why

@page-down
Copy link
Contributor Author

It looks like the window size does not need to change when switching between certain layouts.

For example, three windows, switching between tall:mirrored=false,tall:mirrored=true.

kitty --config=NONE -o enabled_layouts=tall:mirrored=false,tall:mirrored=true -o cursor_blink_interval=0 -o allow_remote_control=yes bash -c 'for i in {1..3}; do kitty @ launch --title win-$i read; done'
diff --git a/kitty/window.py b/kitty/window.py
index 4e256a67..6e0a7d0a 100644
--- a/kitty/window.py
+++ b/kitty/window.py
@@ -551,16 +551,20 @@ def set_geometry(self, new_geometry: WindowGeometry) -> None:
         if self.destroyed:
             return
         if self.needs_layout or new_geometry.xnum != self.screen.columns or new_geometry.ynum != self.screen.lines:
+            log_error(f'window: {self.title} resize')
             self.screen.resize(new_geometry.ynum, new_geometry.xnum)
             sg = self.update_position(new_geometry)
             self.needs_layout = False
             call_watchers(weakref.ref(self), 'on_resize', {'old_geometry': self.geometry, 'new_geometry': new_geometry})
         else:
+            log_error(f'window: {self.title} update position')
             sg = self.update_position(new_geometry)
         current_pty_size = (
             self.screen.lines, self.screen.columns,
             max(0, new_geometry.right - new_geometry.left), max(0, new_geometry.bottom - new_geometry.top))
+        log_error(f'window: {self.title} current_pty_size: {current_pty_size} last: {self.last_reported_pty_size}')
         if current_pty_size != self.last_reported_pty_size:
+            log_error('sending SIGWINCH')
             get_boss().child_monitor.resize_pty(self.id, *current_pty_size)
             if not self.pty_resized_once:
                 self.pty_resized_once = True
switching layout to tall:mirrored=true
window: win-1 update position
window: win-1 current_pty_size: (39, 80, 640, 663) last: (39, 80, 640, 663)
window: win-2 update position
window: win-2 current_pty_size: (19, 80, 640, 323) last: (19, 80, 640, 323)
window: win-3 update position
window: win-3 current_pty_size: (20, 80, 640, 340) last: (20, 80, 640, 340)

I confirmed that after trying to force resize with the following patch, the problem no longer occurs. Please check if there is a better way to handle this, thank you.

diff --git a/kitty/tabs.py b/kitty/tabs.py
index cbef1aed..8dbd79ae 100644
--- a/kitty/tabs.py
+++ b/kitty/tabs.py
@@ -253,6 +253,10 @@ def next_layout(self) -> None:
             else:
                 idx = -1
             nl = self.enabled_layouts[(idx + 1) % len(self.enabled_layouts)]
+            log_error(f'switching layout to {nl}')
+            # DEBUG: force resize
+            for window in self.windows:
+                window.needs_layout = True
             self._set_current_layout(nl)
             self.relayout()

@kovidgoyal
Copy link
Owner

See if
e18b0cd
fixes it.

@page-down
Copy link
Contributor Author

See if e18b0cd fixes it.

Yes, the window layout is re-rendered immediately after switching.

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

No branches or pull requests

2 participants