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

Add flag for centring mouse when activating window (with tests) #31

Merged
merged 9 commits into from
Feb 12, 2018

Conversation

mkropat
Copy link
Owner

@mkropat mkropat commented Feb 12, 2018

Extension on #29. See that PR for discussion.

Jānis Mezītis and others added 9 commits January 31, 2018 19:33
Apparently the shellcheck defaults have changed over the past couple years. Let's just disable shellcheck on jumpapp for now, since so many of the rules are failing.
The previous tests were well written, however they were testing helper functions directly and the integration between that function and the rest of the script didn't appear to be tested. I wasn't initially sure if I was going to break something by moving where the `head` call was being made, so I re-wrote the tests to give me more confidence in the change.
I was actually quite surprised that bash -eq can compare hex strings.
Having not looked at the code for a while, I was confused why we were returning both get_most_recently_focused_window and get_next_window. I think moving up the active window check to the same function and adding a comment makes it a little clearer why we're (potentially) doing both.
After mulling it over, I think "center" is a better mnemonic for this operation than "mouse". I would go with "-c" but it's already in use.

My secondary motivation is that a different -m functionality has already been requested in #11. It hasn't been implemented yet, but I could see it being implemented some day.
If we avoid spawning processes for stuff we can do in bash, we'll have a better chance of keeping jumpapp responsive.
@mkropat mkropat merged commit 66e327f into master Feb 12, 2018
@mkropat mkropat deleted the center-mouse branch February 12, 2018 01:32
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.

1 participant