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

feat(cli): add cycle-layout command #556

Merged
merged 7 commits into from
Oct 26, 2023

Conversation

krshrimali
Copy link
Contributor

@krshrimali krshrimali commented Oct 8, 2023

Resolves issue: #555 (comment).

This PR:

  • Adds command to support Cycle(CycleDirection) using cycle_next and cycle_previous on DefaultLayout
  • As per discussion, this PR will only support cycling layouts for DefaultLayout and not for CustomLayout. feat(cli): add cycle-layout command #556 (comment) <- Discussion link

@krshrimali krshrimali changed the title FEAT: Adding a command to toggle_layout FEAT: Adding a command to support cycling between available layouts Oct 8, 2023
@krshrimali krshrimali marked this pull request as ready for review October 8, 2023 15:05
@krshrimali
Copy link
Contributor Author

@LGUG2Z - Thanks for the initial review, please feel free to take a look again, I haven't tested this on my Windows machine but will try doing it in a few hours from now.

Also marked it as ready for review, wanted to wait until we agree that this is something that could be useful.

I also think we should implement a reverse toggle as a next step, to ensure users can switch back to the last used layout, should be fairly straight forward after this lands in.

Thank you for reviewing the PR, appreciate it.

@krshrimali krshrimali requested a review from LGUG2Z October 8, 2023 15:07
@LGUG2Z
Copy link
Owner

LGUG2Z commented Oct 8, 2023

I'm also currently reviewing this from my Macbook 😅 I'll try this out when I'm back at my desktop later today.

For the reverse toggle, I think we can have the cycle_next and cycle_previous methods on DefaultLayout and have the CycleLayout socket message take a CycleDirection argument like the rest of the Cycle commands do: https://github.com/LGUG2Z/komorebi/blob/master/komorebi-core/src/lib.rs#L47

We might as well add the reversing in the same PR since the rest of the Cycle commands go in both directions and it will be nice to have an atomic commit with all of the functionality implemented.

@krshrimali
Copy link
Contributor Author

Was about to commit, sorry for the bugs there. Thanks for the fixes @LGUG2Z! Hopefully I can talk more about this in a video and send more fixes/features. Please let me know if there are any immediate issues that need help btw.

@LGUG2Z
Copy link
Owner

LGUG2Z commented Oct 8, 2023

No problem! I thought it might be a bit late for you with the time difference between Seattle and India. 😅

Thanks for all the work on this; I'll wait for the build and run it for a few days before merging it in- it should be part of the next versioned release 🎉

@krshrimali
Copy link
Contributor Author

No problem! I thought it might be a bit late for you with the time difference between Seattle and India. 😅

Thanks for all the work on this; I'll wait for the build and run it for a few days before merging it in- it should be part of the next versioned release 🎉

Appreciate it! 🚀 I can't wait to add a keybinding in my config for this. Thanks for being considerate about the timings. 💯

komorebic/src/main.rs Outdated Show resolved Hide resolved
Co-authored-by: Kushashwa Ravi Shrimali <kushashwaravishrimali@gmail.com>
@LGUG2Z LGUG2Z changed the title FEAT: Adding a command to support cycling between available layouts feat(cli): add cycle-layout command Oct 26, 2023
@LGUG2Z LGUG2Z merged commit f79b2e3 into LGUG2Z:master Oct 26, 2023
2 checks passed
LGUG2Z added a commit that referenced this pull request Oct 27, 2023
* Command to ToggleLayout

* Just improve logic of figuring out next layout

* Addr review: rename to "Cycle" instead of toggle, and add a small comment

* As per review comments, implement cycle method on DefaultLayout

* I forgot to remove this, my bad

* feat(cli): fixups for cycle-layout cmd

* Update komorebic/src/main.rs

Co-authored-by: Kushashwa Ravi Shrimali <kushashwaravishrimali@gmail.com>

---------

Co-authored-by: LGUG2Z <jadeiqbal@fastmail.com>
Co-authored-by: جاد <LGUG2Z@users.noreply.github.com>
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.

2 participants