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

fix #2383 for numbered workspaces #2393

Merged
merged 5 commits into from
Aug 28, 2016
Merged

Conversation

bebehei
Copy link
Contributor

@bebehei bebehei commented Jul 9, 2016

No description provided.

@bebehei
Copy link
Contributor Author

bebehei commented Jul 9, 2016

this fixes #2383 for numbered workspaces. It still switches for named workspaces in a different way as the documentation suggests.

For any extra details, consider reading #2383.

Travis Job 1 failed because of docker. Could you please restart it?

@stapelberg
Copy link
Member

I’ll wait for @Airblader’s assessment on whether this is a correct fix for issue #2383, but aside from that, could you please modify the commit description to describe what you’re changing instead of just referencing the issue? We’d like to have the git history self-contained.

Thanks!

@bebehei bebehei force-pushed the fix-workspace-next_prev branch 2 times, most recently from 4353c3e to 27ac02b Compare July 31, 2016 18:10
@bebehei
Copy link
Contributor Author

bebehei commented Jul 31, 2016

  • tests added and enhanced (found an uncovered bug)
    • The test configuration may not see straightforward, but it covers most corner cases now.
  • rebased
  • @Airblader's OK.

@Airblader
Copy link
Member

@stapelberg I'm having trouble verifying this because from my point of view we haven't really decided on the correct behavior in the ticket, have we? I'd actually sort of refer to your judgement of how these commands should operate, I don't have a strong opinion about it… :-)

@bebehei
Copy link
Contributor Author

bebehei commented Aug 4, 2016

@stapelberg I'm having trouble verifying this because from my point of view we haven't really decided on the correct behavior in the ticket, have we? I'd actually sort of refer to your judgement of how these commands should operate, I don't have a strong opinion about it… :-)

Quoting @stapelberg #2383:

It’s intentional that numbered workspaces are sorted, whereas named workspaces are kept in the order in which they are created, both in i3bar and when switching workspaces using workspace next.

Isn't this a clear enough definition? Or do you need to understand my changes? Shall I elaborate them?

@Airblader
Copy link
Member

Ah, I must've indeed missed that somehow. It looks good to me, mostly going off the tests here. :-)

@Airblader
Copy link
Member

@stapelberg Should we go ahead with this?

@stapelberg
Copy link
Member

Sure.

@bebehei Could you rebase please?

remove goto statement to similarize workspace_next and workspace_prev
Enhancing test 528 to test workspace_next and workspace_prev
- Adding tests for worksace_prev
- Mixing workspace distribution over outputs
@bebehei
Copy link
Contributor Author

bebehei commented Aug 24, 2016

@stapelberg rebased.

@stapelberg stapelberg merged commit 18183b8 into i3:next Aug 28, 2016
@Airblader Airblader modified the milestone: 4.13 Sep 25, 2016
@bebehei bebehei deleted the fix-workspace-next_prev branch April 13, 2020 12:12
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.

3 participants