Add "focus_wrapping" option #2953
Conversation
CyberShadow
force-pushed
the
focus_wrapping
branch
2 times, most recently
from
6740654
to
72f6473
Compare
|
The change looks good to me. I'm not a huge fan of having both @stapelberg How do you feel about that? We could do what we did in other places and add |
Sounds good! |
| # the configuration. | ||
| # Ticket: #2352 | ||
| # Bug still in: 4.14-72-g6411130c | ||
| use List::Util qw(first); |
There was a problem hiding this comment.
Remove this line, you’re not using first
docs/userguide
Outdated
| default behavior so you can navigate to all your windows without having to use | ||
| +focus parent+. | ||
| By default, when in a container with several windows or child containers, the | ||
| first container will be focused when you use +focus down+ / +focus left+ on the |
There was a problem hiding this comment.
I realize you haven’t touched this, but I don’t quite see why we only mention “focus down / focus left”, when any directional focus command might trigger wrapping.
Maybe “try to move focus over the edge of a container” is a more accurate term?
|
Sorry for the delay - I got involved with another project which has been rather time-consuming. I hope to make time to get back to this soon.
What's an example option which has been changed in this way that I can use as a reference? |
|
The special value |
Focus wrapping applies to all kinds of containers, not just tabbed/stacked ones.
CyberShadow
force-pushed
the
focus_wrapping
branch
from
72f6473
to
a7ee581
Compare
|
Thanks, feedback addressed. I wasn't sure if I should add a test for testing the new option names - let me know if I should. |
parser-specs/config.spec
Outdated
| @@ -203,6 +204,11 @@ state MOUSE_WARPING: | |||
| value = 'none', 'output' | |||
| -> call cfg_mouse_warping($value) | |||
|
|
|||
| # focus_wrapping | |||
| state FOCUS_WRAPPING: | |||
| value = word | |||
There was a problem hiding this comment.
Instead of using word, could you enumerate the possible values here? I.e. 1, yes, true, on, enable, active, 0, no, false, off, disable, inactive, force. That way, i3 can produce more useful error messages. Also, that way we can keep using strcmp instead of strcasecmp in CFGFUN(focus_wrapping).
There was a problem hiding this comment.
Done.
Speaking of this, I noticed there doesn't seem to be any validation for boolean options, so e.g. enalbe will be silently parsed to false. I think there is room for improvement there too.
There was a problem hiding this comment.
Agreed! Pull requests welcome.
Allow enabling forced focus wrapping by specifying "focus_wrapping force" in i3's configuration. This syntax supersedes the previous "force_focus_wrapping yes" one, which remains available for backwards compatibility.
CyberShadow
force-pushed
the
focus_wrapping
branch
from
a7ee581
to
50edf49
Compare
This implements enhancement request #2352.
The actual change is trivial:
void tree_next(char way, orientation_t orientation) { - _tree_next(focused, way, orientation, true); + _tree_next(focused, way, orientation, config.focus_wrapping); }The rest is bureaucracy. :)