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

Feature: Move panes directionally #762

Merged
merged 13 commits into from
Oct 19, 2021
Merged

Feature: Move panes directionally #762

merged 13 commits into from
Oct 19, 2021

Conversation

kunalmohan
Copy link
Member

Fix: #164

@kunalmohan kunalmohan marked this pull request as ready for review October 12, 2021 13:09
@kunalmohan
Copy link
Member Author

I've added a separate Move mode which can be toggled on/off with Ctrl+f. When in Move mode, one can see the arrow keys to move the current pane directionally.

Copy link
Member

@imsnif imsnif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, this looks great!

I only have two comments:

  1. I know finding free keybindings is hard, but ctrl-f is command-completion in fish. Can we find another one?
  2. When we resize panes, we need to also send a message to the terminal's pty that it has been resized so that the application inside the pane knows about its new size. We do it like this:
    self.os_api.set_terminal_size_using_fd(
    *pid,
    pane.get_content_columns() as u16,
    pane.get_content_rows() as u16,
    );

@kunalmohan
Copy link
Member Author

I know finding free keybindings is hard, but ctrl-f is command-completion in fish. Can we find another one?

How about Ctrl+h or Ctrl+x or Ctrl+v or Ctrl+b or Ctrl+n?

@imsnif
Copy link
Member

imsnif commented Oct 15, 2021

I think ctrl+h is good. It's an alias for backspace in bash, but I guess one can use backspace as a workaround (or remap it)

@kunalmohan
Copy link
Member Author

@imsnif This keybind_test here is failing. I'm not familiar with this part of the codebase, so any ideas about how I would go about fixing this?

@imsnif
Copy link
Member

imsnif commented Oct 15, 2021

I'm not familiar with those either I'm afraid :/ If it were me I'd try to debug and compare to main, see what they expect and what's going wrong.

Also btw - be sure to test that with the ctrl-h shortcut we can still use backspace - might be awkward otherwise :)

@imsnif imsnif mentioned this pull request Oct 18, 2021
@kunalmohan kunalmohan merged commit d90e3d4 into main Oct 19, 2021
@kunalmohan kunalmohan deleted the move-pane branch October 19, 2021 14:50
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.

Feature: move panes directionally
2 participants