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

Added Tmux Config File: tmux.yaml, + default.yaml cleanup. #362

Merged
merged 3 commits into from
Apr 25, 2021

Conversation

SaintFenix
Copy link
Contributor

closes #225

This implementation of tmux keybindings is based around the idea of using the current Lock mode in place of a dedicated tmux mode. In this .yaml I've switched the lock mode over to Ctrl+b.

Running with this is the theory that in the future there'd be a hidden "tmux mode" in which the user could go into by using the default Ctrl+b keybind of tmux, and we could then move these binds over to the default, and just have them exist when a user knows about and wishes to use tmux binds, but otherwise sees the default Zellij binds and uses those. (I personally prefer the default Zellij Binds.)

(I would love to write a hidden tmux mode to add it, but I have literally no experience in Rust, and can only shell script half decently. But I felt I could help here.)

I've also created a possible keybind for every single tmux action that has a keybind, these were scraped from tmux through opening tmux and running the command <prefix-key> ? which then outputs the entirety of the keybinds for tmux. I have created a justification as to how some were minorly tweaked to work with current Zellij, and what features Zellij may add in the future that activate more of them in this attached justification.txt.

The idea here is to create a keybind for every tmux action, so that when Zellij implements more features that also exist in tmux, they merely have to be uncommented, and possibly renamed.

I've also done a quick look over the default.yaml and cleaned it up. Removing extraneous spaces as needed, and also updating the keybinds in resize mode to not all be h on lines 48-54 of /example/default.yaml at once. The intention seeming there that they wished to be able to resize panes with not just arrow keys, but also with vi directional keybinds. (hjlk)


Lastly in reference #224, I can't seem to find a list of all possible keybinds, and formatting for those, so some of this required some poking around to get everything working so that Zellij was happy with it. The justification.txt notes on lines 79-82 where this was an #issue.

Created a 1:1 Tmux.yaml for Zellij, using the Lock mode, as Tmux doesn't have a lock mode.
@qballer qballer requested a review from a-kenji April 24, 2021 21:15
@SaintFenix SaintFenix changed the title Added Tmux Config File: Tmux.yaml, + Default.yaml cleanup. Added Tmux Config File: tmux.yaml, + default.yaml cleanup. Apr 24, 2021
- action: [Resize: Right,]
key: [Char: 'h', Right, ]
key: [Char: 'l', Right, ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, these are a good catch!

@@ -30,14 +30,14 @@ keybinds:
key: [ Alt: ']',]
locked:
- action: [SwitchToMode: Normal,]
key: [Ctrl: 'g',]
key: [Ctrl: 'c',]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think changing the mode per default is a good idea for now.
At the moment we settled on g for the locked mode. Please do feel free to open a separate issue for a proposed change, so we can discuss it.

Copy link
Contributor Author

@SaintFenix SaintFenix Apr 25, 2021

Choose a reason for hiding this comment

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

This here is the issue though to simulate tmux keybinds, I cannot find a way to have there exist a tmux like prefix keybind that is then followed by the needed tmux-like keys at least as far as I can tell in this yaml layout. I attempted to create a new mode named tmux with that keybind, but the yaml doesn't accept it as a new mode is not listed or not in the keybinds shown at the bottom of the screen, listing this error when loading the yaml:

Deserialisation error: keybinds: unknown variant tmux, expected one of Normal, Locked, Resize, Pane, Tab, Scroll, RenameTab at line 3 column 11

I have two alternative ideas for work-arounds, I'll discuss in the body bellow, but skimming that, I think you see the issue here already.

I honestly didn't care for this workaround either, I thought it looked messy, but was the most friendly currently to a tmux user as they don't have a lock mode, though it does ruin that feature in the process which isn't so good. :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this back to g, this is the default file. This should not have the tmux compatible keybinds.

@a-kenji
Copy link
Contributor

a-kenji commented Apr 25, 2021

Thank you very much! That is very helpful.
Once the default.yaml changes are done, I would like to merge them.
Good eye!

I don't see this in the scope of this change to change the default keybindings! So please do open an issue addressing them, if you think that should be necessary.

Running with this is the theory that in the future there'd be a hidden "tmux mode" in which the user could go into by using the default Ctrl+b keybind of tmux, and we could then move these binds over to the default, and just have them exist when a user knows about and wishes to use tmux binds, but otherwise sees the default Zellij binds and uses those. (I personally prefer the default Zellij Binds.)

I am not sure, if this is the right way to go. Since it should be confusing for a user, or especially a new user. If there is an entire different way to interact with the interface suddenly.

(I would love to write a hidden tmux mode to add it, but I have literally no experience in Rust, and can only shell script half decently. But I felt I could help here.)

Depending on where we end up configuration wise, this could potentially be entirely alleviated by an extra config file with that mode #248 included.
Good feeling, this is great help.

The justification.txt is awesome!

Lastly in reference #224, I can't seem to find a list of all possible keybinds, and formatting for those, so some of this required some poking around to get everything working so that Zellij was happy with it. The justification.txt notes on lines 79-82 where this was an #issue.

Yeah, we want to get the possible keybinds better documented, that is definitely still an issue!

Your configuration showed me still one issue, that I think we want to adress, tmux has the prefix key for a reason - it doesn't want to clash with other keybindings, we tried to make the normal mode as minimal as possible, but we still do clash with other keybindings.It would be good to be able to start up in the locked mode, optionally. So all input is first passed through.

I like your solution for now. But can you split the tmux example please into a file with the actual config tmux.yamland a file that is the current config with comments, maybe something like tmux-overview.yaml, because if we do decide to ship with a default tmux config, it would be good to not confuse people with the extra burden.

@khs26
Copy link
Contributor

khs26 commented Apr 25, 2021

@SaintFenix, this PR looks great to me too. One request would be if you could convert your justification.txt into a series of feature request issues? If you want me to pick it up, I'm happy to do that too!

@SaintFenix
Copy link
Contributor Author

SaintFenix commented Apr 25, 2021

I don't see this in the scope of this change to change the default keybindings! So please do open an issue addressing them, if you think that should be necessary.

Running with this is the theory that in the future there'd be a hidden "tmux mode" in which the user could go into by using the default Ctrl+b keybind of tmux, and we could then move these binds over to the default, and just have them exist when a user knows about and wishes to use tmux binds, but otherwise sees the default Zellij binds and uses those. (I personally prefer the default Zellij Binds.)

I am not sure, if this is the right way to go. Since it should be confusing for a user, or especially a new user. If there is an entire different way to interact with the interface suddenly.

(I would love to write a hidden tmux mode to add it, but I have literally no experience in Rust, and can only shell script half decently. But I felt I could help here.)

Yeah, this is part of the issue that using lock as our tmux keybind gets us to. We lose the lock mode. I can revert that easily enough. However without a mode to borrow the initial keybind from, we can't imitate tmux very well, since all of these keybinds would otherwise just be floating in normal mode, or another mode not suited to them.


Here are my two proposed alternative solutions using the same binds:

  1. I create the pull request with the tmux binds listed under a tmux separate mode, currently they will be useless, and won't load in Zellij, but they will pend issue Feature(Config): Make Modes Configurable #248 in which they should then work as you've mentioned. (I like this solution best.

  2. We can figure out some documentation that allows me to set sequences of keybinds through the yaml so I could do something like this instead:

- action: [DetachClient, Quit,]
   key: [ Ctrl: b ; Char: 'd',]

Having like a semicolon (Or any other character) show that the keys must be pressed in sequential order so that a tmux bind can be imitated. This would solve the issue very neatly, and allow some very complicated actions be passed through to Zellij. I think there is a lot of potential in this solution. (Now documented here #372 )

you mentioned that this is an issue here:

Yeah, we want to get the possible keybinds better documented, that is definitely still an issue!

I'll go ahead and create an issue with that in mind, since these would help this pull request quite a bit.

Good feeling, this is great help.

I'm glad! I'm doing my best here, I'm very new to Git, and all of you guys are just so nice.

The justification.txt is awesome!

👍

I like your solution for now. But can you split the tmux example please into a file with the actual config tmux.yaml and a file that is the current config with comments, maybe something like tmux-overview.yaml, because if we do decide to ship with a default tmux config, it would be good to not confuse people with the extra burden

Can do, I'll get that pushed up shortly.


@SaintFenix, this PR looks great to me too. One request would be if you could convert your justification.txt into a series of feature request issues? If you want me to pick it up, I'm happy to do that too!

I would be happy to. :) Should it to be an omnibus of all of them at once? Or for each separate non-implemented bind? (Where it doesn't exist of course. Since we do have a few requests for some of these features already.) @khs26 ?

Corrections according to the discussion of the pull request, namely separating this idea out into a new yaml entirely called tmux-overview., and creating a tmux.yaml with just the new tmux bindings.
@SaintFenix
Copy link
Contributor Author

Corrections implemented, is this what you meant with the file split @a-kenji ?

@khs26
Copy link
Contributor

khs26 commented Apr 25, 2021

@SaintFenix, this PR looks great to me too. One request would be if you could convert your justification.txt into a series of feature request issues? If you want me to pick it up, I'm happy to do that too!

I would be happy to. :) Should it to be an omnibus of all of them at once? Or for each separate non-implemented bind? (Where it doesn't exist of course. Since we do have a few requests for some of these features already.) @khs26 ?

I think separate requests, if you don't mind. Don't worry too much about duplicates, we can deal with them as we find them. At this stage it's very much about having all the issues recorded.

@@ -30,14 +30,14 @@ keybinds:
key: [ Alt: ']',]
locked:
- action: [SwitchToMode: Normal,]
key: [Ctrl: 'g',]
key: [Ctrl: 'c',]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this back to g, this is the default file. This should not have the tmux compatible keybinds.

example/tmux.yaml Outdated Show resolved Hide resolved
@a-kenji
Copy link
Contributor

a-kenji commented Apr 25, 2021

Yeah, this is part of the issue that using lock as our tmux keybind gets us to. We lose the lock mode. I can revert that easily enough. However without a mode to borrow the initial keybind from, we can't imitate tmux very well, since all of these keybinds would otherwise just be floating in normal mode, or another mode not suited to them.

Ah, no that was totally fine and I think the only reasonable action you could take! I just commented that it is a current limitation of zellij that you have to do it this way atm, and that I would like to alleviate that in the future.

I create the pull request with the tmux binds listed under a tmux separate mode, currently they will be useless, and won't load in Zellij, but they will pend issue #248 in which they should then work as you've mentioned. (I like this solution best.

This should also be reasonable to implement in a timely manner.

We can figure out some documentation that allows me to set sequences of keybinds through the yaml so I could do something like this instead:

At some point some sort of prefix key would actually be nice.

Thanks yeah, that is what I meant with the file split.

@SaintFenix
Copy link
Contributor Author

Ah, no that was totally fine and I think the only reasonable action you could take! I just commented that it is a current limitation of zellij that you have to do it this way atm, and that I would like to alleviate that in the future.

I understand you now. 😄

Sounds all sorts of good, I've pushed up the last requested changes now.

Renamed tmux.yaml to tmux.nonimplemented so that it's not mistaken, and removed an accidental edit in the default.yaml.
@a-kenji
Copy link
Contributor

a-kenji commented Apr 25, 2021

Awesome, thank you! This already helped us a ton and showed us current limitations in a tangible way.

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.

tmux like key bindings option
3 participants