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

New feature: Synchronize text sent to panes #395

Merged
merged 4 commits into from
Apr 28, 2021

Conversation

dantepippi
Copy link
Contributor

Creating a new keybinding "s" on the Pane mode. When sync is enabled text will be sent to all terminals on the current tab.

Closes #302

@dantepippi dantepippi marked this pull request as ready for review April 27, 2021 19:12
Copy link
Contributor

@khs26 khs26 left a comment

Choose a reason for hiding this comment

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

Looks great!

I don't think it needs addressing as part of this PR, but I think there are a couple of future extensions - could you create issues for them?

  1. Controlling which panes in a tab are in sync mode - this could be particularly useful if one of the panes is a plugin, which wouldn't expect to handle the same input as a regular pane.
  2. Highlighting which panes are in sync mode (with a green border, like the active pane).

I particularly like that you've considered the UI by adding the "Sync" label to the tab.

@a-kenji
Copy link
Contributor

a-kenji commented Apr 28, 2021

Thank you, this looks great!
The default keybindings are now solely defined under /assets/config/default.yaml

@dantepippi
Copy link
Contributor Author

Looks great!
Thanks for the feedback and taking your time to review my code!

I don't think it needs addressing as part of this PR, but I think there are a couple of future extensions - could you create issues for them?

1. Controlling which panes in a tab are in sync mode - this could be particularly useful if one of the panes is a plugin, which wouldn't expect to handle the same input as a regular pane.

So far I don't replicate the text on plugin panes, only to regular panes. How do you imagine the UI for selecting which panes are in sync? I think being able to sync all panes at once is a nice feature and should also be a choice.

2. Highlighting which panes are in sync mode (with a green border, like the active pane).

I particularly like that you've considered the UI by adding the "Sync" label to the tab.

@dantepippi
Copy link
Contributor Author

Thank you, this looks great!
The default keybindings are now solely defined under /assets/config/default.yaml

I have updated the code to reflect this change. Thank you for bringing it up.

@TheLostLambda
Copy link
Member

Hey @dantepippi! This is a super sick change that's got great code and adds a great feature! Thanks for fixing the merge conflicts!

I'll merge this now and add it to the changelog for you 🙂

Thanks for the awesome PR!

@TheLostLambda TheLostLambda merged commit 3c431ee into zellij-org:main Apr 28, 2021
@dantepippi dantepippi deleted the sync_panes_issue_302 branch April 29, 2021 00:36
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.

Implement functionality propagate (sync) all panes either type or paste text
4 participants