When moving to next or prev workspace, don't skip colliding number #4451
Conversation
This fixes an issue where, having workspaces: 1, 2:foo, 2:bar, and 3, moving from 2:foo to next moves us to workspace 3, and moving from 2:bar to prev moves us to workspace 1. With this change, moving from 2:foo to next would move us to 2:bar, and from 2:bar to prev would move us to 2:foo.
|
Closes #4452 |
|
I just noticed that besides |
|
Change has been extended to |
src/workspace.c
Outdated
| next = child; | ||
| goto workspace_next_on_output_end; |
There was a problem hiding this comment.
| next = child; | |
| goto workspace_next_on_output_end; | |
| return child; |
There was a problem hiding this comment.
Ditto, 19 lines below. This function follows a single-return style. It seems the previous ones were also originally written in such a style.
Personally, I find the assignment clearer. It allows one to visually scan the function source for next = in order to determine where next is determined. With this change, one would have to pay a bit more attention that return child also determines what the next is. It breaks the overall consistency.
src/workspace.c
Outdated
| prev = child; | ||
| return prev; |
There was a problem hiding this comment.
| prev = child; | |
| return prev; | |
| return child; |
src/workspace.c
Outdated
| next = child; | ||
| return next; |
There was a problem hiding this comment.
| next = child; | |
| return next; | |
| return child; |
There was a problem hiding this comment.
This was to be consistent with the style used 23 lines above. Should I change that as well?
There was a problem hiding this comment.
Yes, please change them. Early returns are preferable.
src/workspace.c
Outdated
| prev = child; | ||
| goto workspace_prev_on_output_end; |
There was a problem hiding this comment.
| prev = child; | |
| goto workspace_prev_on_output_end; | |
| return child; |
|
Also, the release notes need to be updated: https://github.com/i3/i3/blob/next/RELEASE-NOTES-next |
Co-authored-by: Orestis Floros <orestisflo@gmail.com>
|
I've updated the release notes. |
This fixes an issue where, having workspaces: 1, 2:foo, 2:bar, and 3,
moving from 2:foo to next moves us to workspace 3, and moving from 2:bar
to prev moves us to workspace 1.
With this change, moving from 2:foo to next would move us to 2:bar, and
from 2:bar to prev would move us to 2:foo.
Closes #4452