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

tmux 2.1 breaks select-pane [-LDUR] when in maximized pane #150

Closed
atweiden opened this issue Oct 20, 2015 · 30 comments
Closed

tmux 2.1 breaks select-pane [-LDUR] when in maximized pane #150

atweiden opened this issue Oct 20, 2015 · 30 comments

Comments

@atweiden
Copy link

After updating tmux to version 2.1, the options for switching panes when in a maximized pane are limited to the following:

select-pane -t:.+
select-pane -t:.-
select-pane -t:.1 # works by pane number
select-pane -l

The rest fails to work when the current pane is maximized:

select-pane -L
select-pane -D
select-pane -U
select-pane -R
@nicm
Copy link
Member

nicm commented Oct 20, 2015

Yeah it was crap to always unzoom so now we don't but that means that -LDUR can't see the layout. Try this:

diff --git a/cmd-select-pane.c b/cmd-select-pane.c
index e76587c..7986e98 100644
--- a/cmd-select-pane.c
+++ b/cmd-select-pane.c
@@ -120,14 +120,19 @@ cmd_select_pane_exec(struct cmd *self, struct cmd_q *cmdq)
        return (CMD_RETURN_NORMAL);
    }

-   if (args_has(self->args, 'L'))
+   if (args_has(self->args, 'L')) {
+       server_unzoom_window(wp->window);
        wp = window_pane_find_left(wp);
-   else if (args_has(self->args, 'R'))
+   } else if (args_has(self->args, 'R')) {
+       server_unzoom_window(wp->window);
        wp = window_pane_find_right(wp);
-   else if (args_has(self->args, 'U'))
+   } else if (args_has(self->args, 'U')) {
+       server_unzoom_window(wp->window);
        wp = window_pane_find_up(wp);
-   else if (args_has(self->args, 'D'))
+   } else if (args_has(self->args, 'D')) {
+       server_unzoom_window(wp->window);
        wp = window_pane_find_down(wp);
+   }
    if (wp == NULL)
        return (CMD_RETURN_NORMAL);

@atweiden
Copy link
Author

Thanks for the patch.

@indiesquidge
Copy link

I am using Homebrew to handle my tmux package. I have been reading about creating patches for tmux, and I don't know if I fully understand what to do with this diff to fix the issue. I have created the diff, but every time I run brew install --interactive --git tmux and make the diff changes, and git diff >> /usr/local/Library/Formula/tmux.rb I get Error: Empty installation. What am I missing in order to get this functionality back?

I have downloaded the Tmux 2.0 tar file and gotten that to work manually based on the readme, but I would rather just patch this one thing to 2.1. Any help would be appreciated.

@nicm
Copy link
Member

nicm commented Oct 21, 2015

I don't know anything about Homebrew you will need to ask them or read
their docs, or install tmux from source yourself.

On Tue, Oct 20, 2015 at 05:06:18PM -0700, Austin Wood wrote:

I am using Homebrew to handle my tmux package. I have been reading about
creating patches for tmux, and I don't know if I fully understand what to
do with this diff to fix the issue. I have [1]created the diff, but every
time I run brew install --interactive --git foo and make the diff changes,
I get Error: Empty installation.

--
Reply to this email directly or [2]view it on GitHub.

Reverse link: [3]unknown

References

Visible links

  1. https://github.com/Homebrew/homebrew/blob/master/share/doc/homebrew/Formula-Cookbook.md#creating-the-diff
  2. tmux 2.1 breaks select-pane [-LDUR] when in maximized pane #150 (comment)
  3. tmux 2.1 breaks select-pane [-LDUR] when in maximized pane #150 (comment)

@rakkesh
Copy link

rakkesh commented Oct 21, 2015

@indiesquidge tmux recipe for Homebrew with the above patch - https://gist.github.com/rakkesh/0cab865147bfac9f195e

@indiesquidge
Copy link

@rakkesh thanks! I did end up figuring out how to create the diff, it's getting it to work when I brew install tmux afterwards. I can now get tmux to install, but it doesn't seem to be picking up on this patch; I am still having trouble with being able to change panes while zoomed. My diff looks very similar to yours, only I think you meant to have __END__ rather than __DATA__ at the beginning of the change.

I don't understand what I am missing or doing incorrectly at this point.

@atweiden, could you expand on how you used this diff to fix the issue? I'm not sure if you downloaded the source and mutated that itself, or if you also were using Homebrew, but I would love to hear your solution :)

@rakkesh
Copy link

rakkesh commented Oct 21, 2015

Indeed, __DATA__ was a typo in the gist which I had linked above.

As for installing tmux with the patch, do

$ brew install --build-from-source tmux

@indiesquidge
Copy link

@rakkesh, yep, that did it! I appreciate your help ver much, thank you :)

Depending on @nicm's preference, I believe this issue can be closed.

@nicm
Copy link
Member

nicm commented Oct 22, 2015

Applied now

@PunkPhysicist
Copy link

If you are using key bindings, you can modify the command to still work (rather than patching the tmux install). For instance to move down a pane, I originally had in my tmux.conf:
bind j select-pane -D
after the update I changed this to
bind j select-pane -D; resize-pane -Z; select-pane -D; resize-pane -Z

What this does is it will try to move down a pane, zoom (or unzoom), try to move down the pane again, and then undo the zoom/unzoom. Since you can't move when zoomed, you only change panes once, and if you are zoomed, you stay zoomed (but now focused on a different pane).

@indiesquidge
Copy link

@PunkPhysicist, that's a neat little hack, thanks for sharing it. The only issue I can see is that it will not zoom out when you change panes while zoomed. What if you aren't sure which pane you would like to change to next? You would have to either keep cycling through the panes until you get to the one you want, or zoom out to find it. Then we are back to the original problem of having to zoom out prior to switching.

It does seem like a neat work around though. I think I am just conditioned to expect my window to zoom out when I switch panes. I have submitted a patch to Homebrew for their Tmux bottle and it has been merged, so now we can have it both ways :)

@PunkPhysicist
Copy link

@indiesquidge I agree, I'd prefer the default behavior to unzoom, then move. As far as I can tell there is only a single zoom/unzoom command. If there was a seperate unzoom (only) command, one could just make the full command "unzoom; move."

@leehambley
Copy link

Can I interject and see if I understood this issue correctly, because I think that I've been fruitlessly trying to replicate what is defined here as "broken" behaviour?

Specifically I want select-pane -{L,D,U,R} to do nothing when I am zoomed, and be forced to manually zoom out. I have something like the following in my config:

is_vim='echo "#{pane_current_command}" | grep -iqE "(^|\/)g?(view|n?vim?)(diff)?$"'
bind -n C-h if-shell "$is_vim" "send-keys C-h" "select-pane -L"
bind -n C-j if-shell "$is_vim" "send-keys C-j" "select-pane -D"
bind -n C-k if-shell "$is_vim" "send-keys C-k" "select-pane -U"
bind -n C-l if-shell "$is_vim" "send-keys C-l" "select-pane -R"

But, I can't reproduce the same issue as here. Interestingly a colleague has the same version of tmux (2.1, via homebrew bottle) on his Mac, and for him he is seeing what here is reported as "broken", and he likes it that way too.

@indiesquidge
Copy link

@leehambley, "broken" is subjective, sure. The point was that it changed from what Tmux had prior to 2.1, and that upset a good number of people who liked the old feature (myself included). I liked having the ability to change panes with select-pane -{L,D,U,R} even when zoomed in. If you prefer to have it do nothing when zoomed in, then this update is perfect for you :)

@leehambley
Copy link

If you prefer to have it do nothing when zoomed in, then this update is perfect for you :)

Hah, well indeed, I'm surprised though, since I am on 2.1, and I still get switched even when I'm zoomed in. I have to take a bit more of a look and find our what commit ref I am running (a bottled brew install, so a real release, I imagine) - at least I know i'm not insane :)

@glentakahashi
Copy link

Bumping this to agree with @leehambley Somehow on one of my machines tmux won't move me when i'm zoomed but will on other machines.

Is there any way to replicate the "broken" behavior? Maybe writing some sort of if-script to check if the pane is maximized?

@leehambley
Copy link

I looked at submitting a patch to make this configurable... Kudos to the authors the code of tmux is VERY clean, but I'm still too much of a cowboy to submit anything useful, I'm afraid :(

@nicm
Copy link
Member

nicm commented Dec 3, 2015

This will not be configurable.

@glentakahashi
Copy link

@leehambley I guess its up to you and me now, :D That's the beauty of open source, time to make a fork of tmux 2.1? Seems like we can just revert the above commit in the fork and all will be fine and dandy for us. Or even better we can just change our homebrew to revert the diff as well.

@leehambley
Copy link

I'm not sure who @nicm and what authority he has to say whether or not this makes sense to be configurable.

@NCIM assuming we have to get it past you, couldn't we even discuss making an option, or something that can be set, but sensibly defaulted like :setw disable-set-pane-when-zoomed (naming things isn't my strong suit ;-))

Forking it does seem like a bit of a nightmare, perhaps we could figure out how to make brew build this with a patch included, and go from there, at least that'd allow us to muck around and see if we could make a patch that kept the best of both worlds.

@glentakahashi
Copy link

@leehambley brew install glentakahashi/homebrew-tmux-no-auto-unzoom/tmux --build-from-source

:)

@leehambley
Copy link

❤️ so, that solves my problem! If I get the urge to write some C I might prep a PR, or at least maintain a patch as a gist, and write a blog entry, nice work :) Thanks!

@nicm
Copy link
Member

nicm commented Dec 3, 2015

The change in 2.1 was unintentional. Most everything else unzooms when needed rather than ignoring the command so this should too. And you can script the 2.1 behaviour with if-shell -F in any case. No more options for minor stuff like this, we have too many already.

@leehambley
Copy link

you can script the 2.1 behaviour with if-shell -F in any case.

That'd be ideal, how do I query for tmux being zoomed though? I had no idea that was possible, but that would be the ideal case, the scriptable nature of tmux solves that usecase perfectly without having to maintain a (doomed) fork.

@nicm
Copy link
Member

nicm commented Dec 3, 2015

There is a window_zoomed_flag format or something like that, it may only be in git though

@glentakahashi
Copy link

@nicm @leehambley

updated code, no need for a fork/patched homebrew
is_zoomed='echo "#{window_zoomed_flag}" | grep 1'
is_vim='echo "#{pane_current_command}" | grep -iqE "(^|/)g?(view|n?vim?)(diff)?$"'
bind -n C-h if-shell "$is_vim || $is_zoomed" "send-keys C-h" "select-pane -L"
bind -n C-j if-shell "$is_vim || $is_zoomed" "send-keys C-j" "select-pane -D"
bind -n C-k if-shell "$is_vim || $is_zoomed" "send-keys C-k" "select-pane -U"
bind -n C-l if-shell "$is_vim || $is_zoomed" "send-keys C-l" "select-pane -R"
bind -n C-\ if-shell "$is_vim || $is_zoomed" "send-keys C-" "select-pane -l"

@leehambley
Copy link

Thanks @glentakahashi I'm wondering how 'echo "#{window_zoomed_flag}" works, but I suppose it exists in the ENV of tmux processes, that was the missing piece for me, thanks!

@jhwang7628
Copy link

Thanks @glentakahashi. Do I just copy and paste it in config file and source? It does not work for me though.

@glentakahashi
Copy link

https://github.com/glentakahashi/dotfiles/blob/master/tmux.conf#L42

Yea you should be able to do this. Could be using a different version of Tmux or something? @jhwang7628

@lock
Copy link

lock bot commented Feb 16, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Feb 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

8 participants