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

Directionalfocus #27

Merged
merged 8 commits into from Feb 19, 2017

Conversation

Projects
None yet
2 participants
@allora
Copy link
Contributor

allora commented Feb 18, 2017

This adds support for cardinal window focusing. Users are able to pick a window to focus based on the closest window within the desired cardinal direction.

For example a user sending the command: waitron window_cardinal_focus up will change the focused window to the one above the current window, if one exists and there is a current focused window. Testing on a triple head setup showed that the algorithm for window focus selection worked as expected across displays.

I've attempted to have "good defaults" for the window selection heuristics, but arguably they could be made into config settings instead, with a default in the config.h file.

I also added a group_remove_all_windows, which does what it sounds like it does. Removes all windows from a group.

@tudurom

This comment has been minimized.

Copy link
Owner

tudurom commented Feb 18, 2017

Wow, I am truly impressed. Thank you for the huge contribution. But there's a problem that isn't letting me merge the pr: you used a different code style. Please adapt your changes to the code style (opening brace on the same line as the statement, no C++ style comments etc.).

@allora

This comment has been minimized.

Copy link
Contributor

allora commented Feb 18, 2017

Sorry about that, work habits :) Updated.

@tudurom

This comment has been minimized.

Copy link
Owner

tudurom commented Feb 19, 2017

Also, the group_remove_all_windows command removes all the windows from all the groups because there is no check. Your code iterates over all windows and removes the group.

@allora

This comment has been minimized.

Copy link
Contributor

allora commented Feb 19, 2017

Updated. This is a total brain fart on my end O_o. In testing it, I don't think it was ever tested with multiple groups assigned haha.

@tudurom

This comment has been minimized.

Copy link
Owner

tudurom commented Feb 19, 2017

Thanks! I will add you to the "thanks to" section in the readme ;)

@tudurom tudurom merged commit 6f99fe3 into tudurom:master Feb 19, 2017

@allora

This comment has been minimized.

Copy link
Contributor

allora commented Feb 19, 2017

Awesome! Thanks :)

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