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

Update workspaces.cpp #2507

Merged
merged 1 commit into from Sep 13, 2023
Merged

Update workspaces.cpp #2507

merged 1 commit into from Sep 13, 2023

Conversation

hariseldon78
Copy link
Contributor

Fix unchecked string to int conversion of workspace name (which can be a string)
Closes #2501

Fix unchecked string to int conversion of workspace name (which can be a string)
Closes Alexays#2501
@Alexays Alexays merged commit 4d32991 into Alexays:master Sep 13, 2023
@Alexays
Copy link
Owner

Alexays commented Sep 13, 2023

Thanks!

@@ -448,7 +447,7 @@ void Workspaces::sort_workspaces() {
return is_name_less;
case SORT_METHOD::NUMBER:
try {
return is_number_less;
return std::stoi(a->name()) < std::stoi(b->name());
Copy link
Contributor

@zjeffer zjeffer Sep 13, 2023

Choose a reason for hiding this comment

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

I think this can be improved more by changing the 3 helper comparisons to be lambdas instead of boolean variables.

Like this:

auto is_id_less = [a, b]() { a->id() < b->id(); };
...

We can then easilly use:

return is_number_less() (with the brackets, because it's a function)

and the try catch would still work without having to put the entire comparison method inside the switch statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. I'm a bit rusty on c++ so i'm quite slow, but i want to make a PR to the Hyprland project enabling persistent workspaces from the compositor itself (if somebody don't beat me to it, it's all very active!), so i'll probably also take a better look at this part of waybar if there is space for improvement.
That fix I made was done in a real rush!

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: module gets disabled.
3 participants