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

attempt at fixing #309 layout issues with tmux 2.6 #312

Merged
merged 2 commits into from Nov 10, 2017

Conversation

Projects
None yet
3 participants
@tony
Member

tony commented Nov 9, 2017

even after setting -x and -y with new-session, select-layout won't take
effect. However, it is necessary in 2.6 to get pane creation in detached
mode to work.

the only way found for tmux select-layout to take effect and prior
behavior to have viewed the window you are running "select-layout"
for in the client.

a solution has been devised to connect the client to the session, then
run select-layout on each window in the session. However, it's still not
enough to load the session, it seems the window must be selected by the client
at least once for the select-layout to take effect.

so a hook against the session is created for the session that selects each
window, runs select layout at the -t target for the window id. and after
that, the hook is neatly unset when that's finished.

attempt at fixing #309 layout issues with tmux 2.6
even after setting -x and -y with new-session, select-layout won't take
effect. However, it is necessary in 2.6 to get pane creation in detached
mode to work.

the only way found for tmux select-layout to take effect and prior
behavior to have viewed the window you are running "select-layout"
for in the client.

a solution has been devised to connect the client to the session, then
run select-layout on each window in the session. However, it's still not
enough to load the session, it seems the window must be selected by the client
at least once for the select-layout to take effect.

so a hook against the session is created for the session that selects each
window, runs select layout at the -t target for the window id. and after
that, the hook is neatly unset when that's finished.

@tony tony referenced this pull request Nov 9, 2017

Closed

layout is broken on 1.3.4 #309

@kageurufu

This comment has been minimized.

kageurufu commented Nov 9, 2017

This brings 2.6 inline with the behavior I get from 2.5 and below, #309 (comment)

Remove -t from selectl in hook
Per the feedback from:

- #309 (comment)
- #312 (comment)

and verified by me with:

- #309 (comment)

main-pane-width/height wasn't being respected when using targets with
selectl.
@tony

This comment has been minimized.

Member

tony commented Nov 9, 2017

@kageurufu does 6e08e03 help?

@kageurufu

This comment has been minimized.

kageurufu commented Nov 9, 2017

That seems to fix main-pane-width/height, I get a proper main pane of 35 rows/cols with your example2 from #309

Does tmuxp or tmux assume a default main-pane-width of 80 when not defined? That would explain the 77/2 split for a new two-pane session as well

@tony

This comment has been minimized.

Member

tony commented Nov 9, 2017

Does tmuxp or tmux assume a default main-pane-width of 80 when not defined? That would explain the 77/2 split for a new two-pane session as well

I don't see any behavior defined as a default for main-pane-height and main-pane-width. I think it's best to set it explicitly.

Between #312 (including 6e08e03) and setting your main-pane-width, does this solve the issues for you pertaining to tmuxp loading the layout correctly?

@kageurufu

This comment has been minimized.

kageurufu commented Nov 9, 2017

I think this meets my expected behavior, but I think we should wait for @oblitum to verify

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

@kageurufu 👍

I'm going to merge this into master for now, then try to clean up other CI issues that cropped up (#304).

When you get back @oblitum give this PR (or master) a shot

@tony tony merged commit 7f1d077 into master Nov 10, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

I tested this now, this doesn't solve the problem for me. The way I'm testing it is by updating from AUR package to 1.3.4 and replacing cli.py with the one from this pull, I also delete python caches from my system.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

For me, the issue is on libtmux, not tmuxp. I've just run tmuxp 1.3.4 with libtmux 0.7.4, instead of 0.7.5, and it's all fine. I also run tmuxp 1.3.2 with libtmux 0.7.5, instead of 0.7.4, and the problem persisted. All from AUR, by patching requires.txt, etc.

So downgrading libtmux fix the issue, while changing tmuxp version is irrelevant.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

@tony, should #309 be migrated?

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

@oblitum I believe what you're experiencing. However I haven't been able to duplicate anything between libtmux and this.

Since I replicated the issue and solved it on my end, and also @kageurufu confirmed it, can you try to clear out libtmux and tmuxp completely, and install tmuxp via git?

pip install https://github.com/tony/tmuxp/archive/master.zip

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Uninstalled tmuxp and dependencies from my system, then installed it as requested inside a virtualenv, run it from there, same issue.

Note that I'm always testing it with the basic layout from the original issue #309.

I've then

  • pip uninstall libtmux
  • pip install libtmux==0.7.4
  • vim ~/.pyenv/versions/tmuxp/lib/python3.6/site-packages/tmuxp-1.3.4-py3.6.egg-info/requires.txt
  • changed libtmux==0.7.5 to libtmux==0.7.4
  • it's fixed.
@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

Q: What version of tmux do you have? tmux -V one more time to be sure

Q: Do you launch tmuxp from within tmux?

If you have tmux 2.6, v0.7.4 will probably fail (pane to small). Unattached clients (building clients in detached states) will only be 80x60 cells until attached.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Q: What version of tmux do you have? tmux -V one more time to be sure

2.6 (I think both me and @kageurufu are running ArchLinux, which is currently at this version)

Q: Do you launch tmuxp from within tmux?

Yes

If you have tmux 2.6, v0.7.4 will probably fail (pane to small). Unattached clients (building clients in detached states) will only be 80x60 cells until attached.

It doesn't fail as I've demonstrated, I start the session by running tmuxp load -d ./foo.yaml.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Did you run the tmuxp version you requested me to run, on ArchLinux, and loaded the original basic session like that (tmuxp load -d ./foo.yaml)?

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

Q: Do you launch tmuxp from within tmux?
Yes

That's a big detail

Can you try launching tmuxp from outside tmuxp and see if it works.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

It doesn't fail as I've demonstrated, I start the session by running tmuxp load -d ./foo.yaml.

That's an even bigger detail! I think I know what it could be.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Launching from outside tmux has no effect.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Removing these lines fixes it.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

That link needs to get fixed

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Trying.... hit some github issue in the comment system now.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

@oblitum I bet d37f62b will fix it.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

I meant d37f62b

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Updated the comment. Github doesn't support pasting URLs with #s, has to be inside a link.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

One of the things that makes this interesting is that you're using tmuxp a bit differently. But I think we can support the case.

I am not sure if you need to clean out the old libtmux/tmuxp/etc to test the latest master. But I think this should work.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

but it's not looking as the previous main vertical layout either, the main pane on the left is tighter than the one on the right.

Yeah, I had to make my main-pane-width a LOT bigger. I think recent tmux version changed the calculation somehow.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

Can say whether it's working as it's supposed to. Now for the first time, it launched differently. There isn't a pane squeezed to 2 columns anymore, but it's not looking as the previous main vertical layout either, the main pane on the left is tighter than the one on the right.

And this is outside of tmux, with and without -d, right?

tmuxp load with and without -d, inside tmux, still breaks, right?

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Yes right.

This what I get:

screenshot

But as I said. If I remove those lines with -x/-y arguments from libtmux, the behavior is like before, regardless from where I start.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

But as I said. If I remove those lines with -x/-y arguments from libtmux, the behavior is like before, regardless from where I start.

OK, I have an idea to try tomorrow: If $TMUX environmental variable is set (the tmuxp load command is ran inside of tmux client) we won't set the -x/-y.

Because we kinda need the -x/-y in 2.6 if they're not attached to any clients.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

OK.

I think recent tmux version changed the calculation somehow.

Notice though that @kageurufu reproduced the issue in several tmux versions.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

#312 (comment) seemed to help @kageurufu. It had to do with setting the main pane option.

Underlying defaults we think are intended behavior of tmux, sometimes actually aren't defined officially by tmux. But as a community, we grow accustomed to building our configs around default, yet unofficial behavior. D: Then when underlying options around the behavior or side effects we grown accustomed to change with tmux versions, it gets hard to figure the cause of.

An example @kageurufu faced was main pane sizes being somewhat reasonable despite no option for them being set. But there was never any option set for those, and that fixed with width/height issue for him.

Another example would be tmux calculating the dimensions of windows in sessions despite no client being attached. That behavior is gone in 2.6. You didn't see any issue with this since you launch tmuxp already in a client, but others would have. This is where the -x/-y workaround came from. (Of course, setting -x and -y when already attached to a tmux client is probably a bad idea. I plan on fixing that tomorrow.)

A third example would be, even after manually setting dimensions via new-session, select layout not working. In fact, select layout won't set the window layout until the window is selected in the client. This is drastically different behavior, but it's because tmuxp relied on defaults that weren't defined.

Thankfully, there are hooks. And we automatically set them, and they seem pretty resilient.

Here is the issue, there are many things that could be a libtmux or tmuxp issue. But I have to rule out it being environments, configs, and tmux versions so I can debug it.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

My comment was just trying to point that it has always been fine for all of us up to the -x/-y change got incorporated, so the assumption that the issue is due tmux calculation changes, despite being shown present in 2.3, 2.4, 2.5 and 2.6, doesn't sound sensible.

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

My comment was just trying to point that it has always been fine for all of us up to the -x/-y change got incorporated, so the assumption that the issue is due tmux calculation changes, despite being shown present in 2.3, 2.4, 2.5 and 2.6, doesn't sound sensible.

I get the point on that and believe you. I'm trying to also say there's a lot of variables in play.

despite being shown present in 2.3, 2.4, 2.5 and 2.6, doesn't sound sensible.

Were you experiencing this in earlier tmux versions? How? the -x/-y thing only effect 2.6. And the calculation comment was about having to increase the number.

The screenshots from @kageurufu were a different thing. Those were more to do with not setting the pane option across all those versions at all. And the behavior in 2.6 was due to the layout not being set if clients were attached.

The other detail is I don't typically run tmuxp with -d or in another tmux session (though I do sometimes), so I don't always catch those. It may be a good idea to make some tests for that.

Here's a second point I want to explain from my end: I just want to be able to find the problem and debug it. I'm not saying it doesn't exist, but if I can't recreate it, and it was an environment/config issue, like not setting panes right, that's better to troubleshoot early

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

Scratch that, I just recalled the -x/-y thing was behind the if has_gte_version('2.6') flag :)

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

No problem.

I'm gonna hit the hay and deal with the -x/-y issue tomorrow

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

And the calculation comment was about having to increase the number.

Ah OK, I got it now.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

So many things at once, wow.

tony added a commit to tmux-python/libtmux that referenced this pull request Nov 10, 2017

tony added a commit that referenced this pull request Nov 10, 2017

tony added a commit that referenced this pull request Nov 10, 2017

bump libtmux to 0.7.7
this has the fix for -x/-y when tmuxp is used inside tmux

related: #312
@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

@oblitum @kageurufu released tmuxp 1.3.5 and libtmux 0.7.7. give it a shot

@kageurufu

This comment has been minimized.

kageurufu commented Nov 10, 2017

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

@kageurufu spectacular

tmux hooks to the rescue. I made a blog post delving into a hooks in greater detail too.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

@kageurufu I'm getting SSL issue with kaptan installation, as someone else commented on the package's AUR page.

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

@tony I just tried 1.3.5/0.7.7 and the layout, even launched from a existing tmux session as I do, is showing up as I commented before, with new sizes and the main pane smaller.

Should I expect now to be explicit about main-horizontal/main-vertical pane sizes or expect for a fix to bring the previous defaults?

@tony

This comment has been minimized.

Member

tony commented Nov 10, 2017

Can we make a separate issue for that?

@oblitum

This comment has been minimized.

oblitum commented Nov 10, 2017

OK.

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