-
-
Notifications
You must be signed in to change notification settings - Fork 957
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
feat: cyclenext, visible parameter #9045
base: main
Are you sure you want to change the base?
Conversation
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.
not against this, but why not just add next visible
and prev visible
to cyclenext
?
as in
bind = SUPER,amogus,cyclenext,visible
bind = SUPER,amogus,cyclenext,prev visible
sounds great, I'll make some edits |
@vaxerski that way - done I'm rereading it and thinking that maybe it would be better to make this behavior the default, and make the workspace cycle optional, and change the function names? |
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.
also am against changing the default behavior. Let's keep this opt-in
if (w != pWindow && w->m_pWorkspace == pWindow->m_pWorkspace && w->m_bIsMapped && !w->isHidden() && (!focusableOnly || !w->m_sWindowData.noFocus.valueOrDefault())) | ||
return w; | ||
} | ||
inline static bool isWorkspaceMatches(PHLWINDOW pWindow, const PHLWINDOW w, bool anyWorkspace) { |
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.
wtf does isWorkspaceMatches
mean? xD
windowAvailableForCycle
? or what?
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.
isWindowAvailableForCycle
is a little bit lower, this is the part of it, isWorkspaceMatches
means window availability checks for cycle, related to workspace
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.
isWorkspaceMatches
is such a confusing name I still don't understand what it's actually doing
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.
Checks workspace for cyclenext.
When visible parameter is passed, checks is workspace visible, otherwise checks is workspace same as current
Describe your PR, what does it fix/add?
#6678
cyclenext, visible
shoud works as cyclenext, but also cycling other monitor windowsIs there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)
nothing
Is it ready for merging, or does it need work?
ready for merging