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 keybindings: prev_output, prev_output_with_win #2072

Merged
merged 2 commits into from Dec 25, 2023

Conversation

A7R7
Copy link
Contributor

@A7R7 A7R7 commented Dec 19, 2023

provide an api wf::output_t *get_prev_output(wf::output_t *output) in core/output-layout.hpp
and use this api to implement prev_output and prev_output_with_win
fixes #2071

@A7R7
Copy link
Contributor Author

A7R7 commented Dec 19, 2023

Haven't noticed there's already #1570. I may take up his work then

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the other PR (#1570) there are two changes needed:

  • The get_prev_output() shouldn't be added to core at all, oswitch can do that on its own.
  • It would be nice to refactor the code so that the functions for next and previous output reuse the same code. Currently, they do the exact same thing but with the only difference of previous/next output. I imagine it should be easy to refactor them to use a common function switch_to_output(target_output) and switch_to_output_with_window(target_output)

@A7R7 A7R7 force-pushed the master branch 2 times, most recently from a23f5cc to 52b4aef Compare December 20, 2023 06:17
@A7R7
Copy link
Contributor Author

A7R7 commented Dec 20, 2023

I did some refactor to the original functions, most importantly add get_output_relative to get the target monitor instead of changing the core api. The 4 activator_callbacks now only use switch_to_output(target_output) and switch_to_output_with_window(target_output) to finish their jobs.

I haven't implement swap_output and swap_output_prev in #1570, because I'm not sure if it is better to swap only the top view or swap the whole active workspace on the 2 outputs, or if it is good to add both functionalities (which I think will be a little mess).

@A7R7 A7R7 requested a review from ammen99 December 20, 2023 06:33
@ammen99
Copy link
Member

ammen99 commented Dec 20, 2023

I haven't implement swap_output and swap_output_prev in #1570, because I'm not sure if it is better to swap only the top view or swap the whole active workspace on the 2 outputs, or if it is good to add both functionalities (which I think will be a little mess).

That is fine, since you are doing the work, you can only add whatever you find useful :)

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thanks!

@ammen99
Copy link
Member

ammen99 commented Dec 20, 2023

Please fix code style as well (there is a diff in the CI logs)

This commit makes get_output_relative get the target output n steps after current output (instead of a given output), in order to reduce duplicated funciton calls in activator callbacks.
@A7R7
Copy link
Contributor Author

A7R7 commented Dec 20, 2023

I've fixed the code style with your fork of uncrustify, hopefully it can pass!

@A7R7 A7R7 requested a review from ammen99 December 21, 2023 00:17
@ammen99 ammen99 merged commit ab9955c into WayfireWM:master Dec 25, 2023
4 checks passed
ammen99 pushed a commit that referenced this pull request Mar 13, 2024
* refactor oswitch functions

* change the logic of get_output_relative

This commit makes get_output_relative get the target output n steps after current output (instead of a given output), in order to reduce duplicated funciton calls in activator callbacks.
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.

Allow keybindings to move to the previous output
2 participants