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

layout toggle split doesn't work until enabling tabbed/stacked mode once #2846

Closed
Streetwalrus opened this Issue Jul 29, 2017 · 27 comments

Comments

Projects
None yet
@Streetwalrus
Contributor

Streetwalrus commented Jul 29, 2017

Output of i3 --moreversion 2>&- || i3 --version:

Running i3 version: 4.13-130-gb23f23b2 (2017-07-13, branch "makepkg") (pid 640)
Loaded i3 config: /home/streetwalrus/.i3/config (Last modified: Fri 28 Jul 2017 19:10:45 IDT, 105654 seconds ago)

The i3 binary you just called: /usr/bin/i3
The i3 binary you are running: i3

URL to a logfile as per http://i3wm.org/docs/debugging.html:

n/a

What I did:

Press $mod+e (bound to layout toggle split) with a top level container focused.

What I saw:

Split orientation doesn't toggle. Switching the container to tabbed or stacked layout fixes the issue for its lifetime. Nested containers are unaffected.

What I expected instead:

Split orientation should toggle no matter what.

@i3bot

This comment has been minimized.

Show comment
Hide comment
@i3bot

i3bot Jul 29, 2017

I don’t see a link to logs.i3wm.org. Did you follow http://i3wm.org/docs/debugging.html? (In case you actually provided a link to a logfile, please ignore me.)

i3bot commented Jul 29, 2017

I don’t see a link to logs.i3wm.org. Did you follow http://i3wm.org/docs/debugging.html? (In case you actually provided a link to a logfile, please ignore me.)

@acrisci

This comment has been minimized.

Show comment
Hide comment
@acrisci

acrisci Jul 29, 2017

Member

Yeah I can reproduce this.

Member

acrisci commented Jul 29, 2017

Yeah I can reproduce this.

@jolange

This comment has been minimized.

Show comment
Hide comment
@jolange

jolange Jul 30, 2017

Contributor

I cannot reproduce this. Maybe I don't understand what you are doing exaclty. Can you give exact instructions (like a sequence of "open terminal" and $mod+e)?

Contributor

jolange commented Jul 30, 2017

I cannot reproduce this. Maybe I don't understand what you are doing exaclty. Can you give exact instructions (like a sequence of "open terminal" and $mod+e)?

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Jul 30, 2017

Contributor

Just open two or more windows on an emtpy workspace then press $mod+e.

Contributor

Streetwalrus commented Jul 30, 2017

Just open two or more windows on an emtpy workspace then press $mod+e.

@jolange

This comment has been minimized.

Show comment
Hide comment
@jolange

jolange Jul 30, 2017

Contributor

Just open two or more windows on an emtpy workspace then press $mod+e.

Works for me on 4.13. I'll try the current dev version later.

Contributor

jolange commented Jul 30, 2017

Just open two or more windows on an emtpy workspace then press $mod+e.

Works for me on 4.13. I'll try the current dev version later.

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Jul 30, 2017

Contributor

Yeah this is the current next branch, it worked fine a month ago before I left.

Contributor

Streetwalrus commented Jul 30, 2017

Yeah this is the current next branch, it worked fine a month ago before I left.

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Jul 30, 2017

Contributor

Just bisected this, the faulty commit is 52ce8c8, confirmed by reverting it at the tip of the branch.

Contributor

Streetwalrus commented Jul 30, 2017

Just bisected this, the faulty commit is 52ce8c8, confirmed by reverting it at the tip of the branch.

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Jul 30, 2017

Contributor

This seems to fix it with no adverse effect on what that commit fixes:

diff --git a/src/con.c b/src/con.c
index 136ce5d2..bf270b24 100644
--- a/src/con.c
+++ b/src/con.c
@@ -1674,7 +1674,7 @@ void con_set_layout(Con *con, layout_t layout) {
             con->workspace_layout = ws_layout;
             DLOG("Setting layout to %d\n", layout);
             con->layout = layout;
-        } else if (layout == L_STACKED || layout == L_TABBED) {
+        } else if (layout == L_STACKED || layout == L_TABBED || layout == L_SPLITV || layout == L_SPLITH) {
             DLOG("Creating new split container\n");
             /* 1: create a new split container */
             Con *new = con_new(NULL, NULL);
Contributor

Streetwalrus commented Jul 30, 2017

This seems to fix it with no adverse effect on what that commit fixes:

diff --git a/src/con.c b/src/con.c
index 136ce5d2..bf270b24 100644
--- a/src/con.c
+++ b/src/con.c
@@ -1674,7 +1674,7 @@ void con_set_layout(Con *con, layout_t layout) {
             con->workspace_layout = ws_layout;
             DLOG("Setting layout to %d\n", layout);
             con->layout = layout;
-        } else if (layout == L_STACKED || layout == L_TABBED) {
+        } else if (layout == L_STACKED || layout == L_TABBED || layout == L_SPLITV || layout == L_SPLITH) {
             DLOG("Creating new split container\n");
             /* 1: create a new split container */
             Con *new = con_new(NULL, NULL);
@jolange

This comment has been minimized.

Show comment
Hide comment
@jolange

jolange Jul 30, 2017

Contributor

Thanks for the bisect. Your fix looks reasonable to me. I'll test it later.

Contributor

jolange commented Jul 30, 2017

Thanks for the bisect. Your fix looks reasonable to me. I'll test it later.

@jolange

This comment has been minimized.

Show comment
Hide comment
@jolange

jolange Jul 30, 2017

Contributor

I can confirm that your patch fixes the issue on the current next branch.
Do you want to create a PR? Maybe a regression test for this would be good, too.

Contributor

jolange commented Jul 30, 2017

I can confirm that your patch fixes the issue on the current next branch.
Do you want to create a PR? Maybe a regression test for this would be good, too.

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Jul 30, 2017

Contributor

I can whip up a PR, I'll have to look into how tests work though. They don't look too hard to write, but I'm not familiar with perl.

Contributor

Streetwalrus commented Jul 30, 2017

I can whip up a PR, I'll have to look into how tests work though. They don't look too hard to write, but I'm not familiar with perl.

hwangcc23 added a commit to hwangcc23/i3 that referenced this issue Aug 6, 2017

Fix 'layout toggle split'
Fix the issue #2846 (i3#2846)

1). Create a new split container when switching a workspace container to split layout
2). Add a regression test in 167-workspace_layout.t

hwangcc23 added a commit to hwangcc23/i3 that referenced this issue Aug 8, 2017

Fix 'layout toggle split'
Fix the issue #2846 (i3#2846)

1). Create a new split container when switching a workspace container to split layout
2). Add one regression test in 167-workspace_layout.t:
    - Get a fresh workspace
    - Set the layout to something
    - Create windows
    - Try to switch to another layout
    - Check if successful
    - Repeat for all 12 possible transitions
3). Add another regression test in 167-workspace_layout.t:
    - Check that the command 'layout toggle split' works regardless of what layout we're

hwangcc23 added a commit to hwangcc23/i3 that referenced this issue Aug 18, 2017

Fix 'layout toggle split'
Fix the issue #2846 (i3#2846)

1). Create a new split container when switching a workspace container to split layout (credit: Streetwalrus)
2). Add one regression test in 167-workspace_layout.t:
    - Get a fresh workspace
    - Set the layout to something
    - Create windows
    - Try to switch to another layout
    - Check if successful
    - Repeat for all 12 possible transitions
3). Add another regression test in 167-workspace_layout.t:
    - Check that the command 'layout toggle split' works regardless of what layout we're

hwangcc23 added a commit to hwangcc23/i3 that referenced this issue Sep 18, 2017

Fix 'layout toggle split'
Fix the issue #2846 (i3#2846)

1). Create a new split container when switching a workspace container to split layout (credit: Streetwalrus)
2). Add one regression test in 167-workspace_layout.t:
    - Get a fresh workspace
    - Set the layout to something
    - Create windows
    - Try to switch to another layout
    - Check if successful
    - Repeat for all 12 possible transitions
3). Add another regression test in 167-workspace_layout.t:
    - Check that the command 'layout toggle split' works regardless of what layout we're
@ianks

This comment has been minimized.

Show comment
Hide comment
@ianks

ianks Sep 18, 2017

@Streetwalrus I have tried your patch and it fixes my issues. 👍

ianks commented Sep 18, 2017

@Streetwalrus I have tried your patch and it fixes my issues. 👍

uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Sep 27, 2017

gahr
x11-wm/i3: import patch from upstream, bump PORTREVISION
The patch fixes i3/i3#2846

Approved by:	bapt (maintainer)


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@450736 35697150-7ecd-e111-bb59-0022644237b5

uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Sep 27, 2017

x11-wm/i3: import patch from upstream, bump PORTREVISION
The patch fixes i3/i3#2846

Approved by:	bapt (maintainer)
@aksr

This comment has been minimized.

Show comment
Hide comment
@aksr

aksr Sep 27, 2017

I'm experiencing the same problem...

aksr commented Sep 27, 2017

I'm experiencing the same problem...

mat813 pushed a commit to mat813/freebsd-ports that referenced this issue Oct 3, 2017

gahr
x11-wm/i3: import patch from upstream, bump PORTREVISION
The patch fixes i3/i3#2846

Approved by:	bapt (maintainer)


git-svn-id: https://svn.freebsd.org/ports/head@450736 35697150-7ecd-e111-bb59-0022644237b5
@ianks

This comment has been minimized.

Show comment
Hide comment
@ianks

ianks Oct 12, 2017

What's the status of this? I just upgraded to 4.14.1 and it doesnt seem to be working

ianks commented Oct 12, 2017

What's the status of this? I just upgraded to 4.14.1 and it doesnt seem to be working

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Oct 12, 2017

Member

The PR for this failed the tests the last time it ran. That's the status.

Member

Airblader commented Oct 12, 2017

The PR for this failed the tests the last time it ran. That's the status.

@lillem4n

This comment has been minimized.

Show comment
Hide comment
@lillem4n

lillem4n Nov 28, 2017

Sooo, why is it closed if it is still an issue?

lillem4n commented Nov 28, 2017

Sooo, why is it closed if it is still an issue?

@orestisf1993

This comment has been minimized.

Show comment
Hide comment
@orestisf1993

orestisf1993 Nov 28, 2017

Member

Fixed with #2849

Member

orestisf1993 commented Nov 28, 2017

Fixed with #2849

@jackodirks

This comment has been minimized.

Show comment
Hide comment
@jackodirks

jackodirks Feb 8, 2018

So, just putting this out there: this fix is committed in october 2017, but the latest release (4.14.1) is from september 2017, meaning that this fix is still not available unless I compile from source. Is there a timeline to release this fix?

jackodirks commented Feb 8, 2018

So, just putting this out there: this fix is committed in october 2017, but the latest release (4.14.1) is from september 2017, meaning that this fix is still not available unless I compile from source. Is there a timeline to release this fix?

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Feb 8, 2018

Member

We don't have a fixed release cycle but typically we release "major" versions about twice a year. It depends a bit on how much has happened.

Member

Airblader commented Feb 8, 2018

We don't have a fixed release cycle but typically we release "major" versions about twice a year. It depends a bit on how much has happened.

@k1-hedayati

This comment has been minimized.

Show comment
Hide comment
@k1-hedayati

k1-hedayati Feb 14, 2018

I have i3 version 4.14.1 (2017-09-24) and waiting for new version to fix this bug

k1-hedayati commented Feb 14, 2018

I have i3 version 4.14.1 (2017-09-24) and waiting for new version to fix this bug

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Feb 14, 2018

Contributor

Complaining won't really get you anywhere. Build next yourself if you really need it.

That said, I'm surprised that bug fixes such as this one aren't being backported to the latest "stable" release.

Contributor

Streetwalrus commented Feb 14, 2018

Complaining won't really get you anywhere. Build next yourself if you really need it.

That said, I'm surprised that bug fixes such as this one aren't being backported to the latest "stable" release.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Feb 14, 2018

Member

That said, I'm surprised that bug fixes such as this one aren't being backported to the latest "stable" release.

They are in the shape of bugfix releases (such as 4.14.1), it's just a matter of how often a bugfix release is made.

That said, we are happy to have more people help test the development branch before a release to catch bugs early on. It's a good and easy way of getting started with contributing :-)

Member

Airblader commented Feb 14, 2018

That said, I'm surprised that bug fixes such as this one aren't being backported to the latest "stable" release.

They are in the shape of bugfix releases (such as 4.14.1), it's just a matter of how often a bugfix release is made.

That said, we are happy to have more people help test the development branch before a release to catch bugs early on. It's a good and easy way of getting started with contributing :-)

@Streetwalrus

This comment has been minimized.

Show comment
Hide comment
@Streetwalrus

Streetwalrus Feb 14, 2018

Contributor

They are in the shape of bugfix releases (such as 4.14.1), it's just a matter of how often a bugfix release is made.

Fair enough.

That said, we are happy to have more people help test the development branch before a release to catch bugs early on. It's a good and easy way of getting started with contributing :-)

I've been tracking next for months and haven't had any trouble with it. I know it does scare a lot of people to run something "potentially unstable" though.

Contributor

Streetwalrus commented Feb 14, 2018

They are in the shape of bugfix releases (such as 4.14.1), it's just a matter of how often a bugfix release is made.

Fair enough.

That said, we are happy to have more people help test the development branch before a release to catch bugs early on. It's a good and easy way of getting started with contributing :-)

I've been tracking next for months and haven't had any trouble with it. I know it does scare a lot of people to run something "potentially unstable" though.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Feb 14, 2018

Member

Yes, I get that too. But for us it's a crucial tool, next to the test suite, to achieve a stable release. So any help there is appreciated and ultimately helps everyone. It's definitely more helpful than "is this still not fixed?", "will this be fixed soon?", "waiting for the fix!" etc comments (as you said as well) ;-)

Member

Airblader commented Feb 14, 2018

Yes, I get that too. But for us it's a crucial tool, next to the test suite, to achieve a stable release. So any help there is appreciated and ultimately helps everyone. It's definitely more helpful than "is this still not fixed?", "will this be fixed soon?", "waiting for the fix!" etc comments (as you said as well) ;-)

@k1-hedayati

This comment has been minimized.

Show comment
Hide comment
@k1-hedayati

k1-hedayati Feb 14, 2018

Complaining won't really get you anywhere. Build next yourself if you really need it.

I'm not complaining. I'm just stating that I have this issue too, hoping for priority to get higher as more users encountering this issue.

It's definitely more helpful than "is this still not fixed?", "will this be fixed soon?", "waiting for the fix!" etc comments (as you said as well) ;-)

I'd spend time to build and test latest i3 version if I was a heavy user but it's not the case at the moment so I just expect it to work, I know about open source philosophy but I'm spending my energy somewhere else.

k1-hedayati commented Feb 14, 2018

Complaining won't really get you anywhere. Build next yourself if you really need it.

I'm not complaining. I'm just stating that I have this issue too, hoping for priority to get higher as more users encountering this issue.

It's definitely more helpful than "is this still not fixed?", "will this be fixed soon?", "waiting for the fix!" etc comments (as you said as well) ;-)

I'd spend time to build and test latest i3 version if I was a heavy user but it's not the case at the moment so I just expect it to work, I know about open source philosophy but I'm spending my energy somewhere else.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Feb 14, 2018

Member

I get the sentiment of "voting" for an issue, but please do so using the voting system. Noise comments trigger emails to all subscribed users. Also, this is already fixed so there is no priority to give to it (not that we do that anyway). You'll jut have to wait for a release.

Member

Airblader commented Feb 14, 2018

I get the sentiment of "voting" for an issue, but please do so using the voting system. Noise comments trigger emails to all subscribed users. Also, this is already fixed so there is no priority to give to it (not that we do that anyway). You'll jut have to wait for a release.

@cpick

This comment has been minimized.

Show comment
Hide comment
@cpick

cpick Aug 16, 2018

For anyone running Ubuntu Bionic, I created a PPA that applies this patch to the 4.14.1-1 version of i3 that bionic ships with:

sudo add-apt-repository ppa:cpick/i3
sudo apt update
sudo apt install i3

cpick commented Aug 16, 2018

For anyone running Ubuntu Bionic, I created a PPA that applies this patch to the 4.14.1-1 version of i3 that bionic ships with:

sudo add-apt-repository ppa:cpick/i3
sudo apt update
sudo apt install i3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment