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: hyprland workspaces add sort-by #2486

Merged
merged 10 commits into from Sep 11, 2023

Conversation

khaneliman
Copy link
Contributor

@khaneliman khaneliman commented Sep 9, 2023

Closes: #2474

Adds a sort-by option that takes in a string to choose how to sort workspaces. This allows you to manually specify the sorting method and future sorting methods can just utilize the same config option. Fallsback to the original sorting.

Copy link
Contributor

@zjeffer zjeffer left a comment

Choose a reason for hiding this comment

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

Looks great! Just suggesting one minor change.

I don't really have time to test this thoroughly right now, so make sure you test:

  • switching between persistent workspaces
  • adding new workspaces by name
  • switching to special workspaces
  • switching to named special workspaces
  • making a (named) special workspace persistent, adding and removing the windows from it, and switching between this workspace and another: the special workspace shouldn't change position if the workspace is empty or not.

include/modules/hyprland/workspaces.hpp Outdated Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
@khaneliman
Copy link
Contributor Author

@zjeffer Updated to use an enum based approach. Sort-by overrides the sorting mechanism and falls back to previous sorting method with a warning if an incorrect value is supplied.

include/util/enum.hpp Outdated Show resolved Hide resolved
@khaneliman
Copy link
Contributor Author

khaneliman commented Sep 9, 2023

@zjeffer new commit with changes to make it easier for other classes to use. There are a few different config related things we could probably create an issue / PR to discuss and brainstorm some more ideas.

Copy link
Contributor

@zjeffer zjeffer left a comment

Choose a reason for hiding this comment

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

Great, just some small suggestions left:

include/util/enum.hpp Outdated Show resolved Hide resolved
include/util/enum.hpp Outdated Show resolved Hide resolved
include/util/enum.hpp Outdated Show resolved Hide resolved
include/modules/hyprland/workspaces.hpp Outdated Show resolved Hide resolved
@khaneliman
Copy link
Contributor Author

Great, just some small suggestions left:

Sounds good, I also realized I now need to make sure the keys are uppercase so i'll add a commit to make that work since people can provide their own keys. Just trying to make it so you dont have to worry about case when a person is writing their config.

@khaneliman
Copy link
Contributor Author

@zjeffer appreciate the thorough review... learned a few things :)

include/util/enum.hpp Outdated Show resolved Hide resolved
@khaneliman
Copy link
Contributor Author

@zjeffer Alright, handled those last couple suggestions.

@khaneliman
Copy link
Contributor Author

@Alexays This one should be ready, now.

@Alexays Alexays merged commit 05a2af2 into Alexays:master Sep 11, 2023
8 checks passed
@Alexays
Copy link
Owner

Alexays commented Sep 11, 2023

LGTM, thx to both of you!

@khaneliman khaneliman deleted the workspace-sorting branch September 11, 2023 13:38
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.

hyprland workspaces sorted differently
3 participants