-
Notifications
You must be signed in to change notification settings - Fork 70
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 Center Window Frame function #121
Conversation
windows.fnl
Outdated
@@ -89,15 +90,39 @@ | |||
(: (hs.window.focusedWindow) :maximize 0) | |||
(highlight-active-window)) | |||
|
|||
(defn position-window-center | |||
[ratio-str window] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the screen should probably be passed in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make this position-window-center-on-screen
, though I think the name now is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about doing that, but was thinking that really depends on the implementation. The default behavior is to use the active screen, but users may want it to cycle between screens or pick the main screen, or any other logic to select a screen. The ratio-str and window are the only facts that are safe to assume for anticipated use-cases.
Though I suppose it could accept the current screen as a default and not use it but if you are aiming to implement advice then it's kind of a waste to have it query the screens again twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By making the screen an arg we assuming less. We can make center-window-frame
provide (hs.screen.primaryScreen)
by default.
By having more of the knobs in the args it also makes advising easier, since you can write one that just modifies the arguments instead of having to duplicate the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have a good point there. Pushed!
Fixes #119
Allows you to add something like the following to your config.fnl:
Then you can use the center-window-frame window modal action to use it like the following:
Creates an advisable function called
windows.position-window-center
that takes the center-ratio config string and the target window. It is responsible for parsing the center-ratio string into numbers, and applying them to the target window. The advisable function gives users the option of overriding it to parse the center-ratio string in a different format and apply the calculation differently if needed.If an option is not set, defaults to
80:50
which is a percentage of the screen. This is nearly identical to the previous version defaults.