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

Fix wrong layout recalculate if statement #1167

Merged
merged 2 commits into from
Dec 4, 2022

Conversation

FlafyDev
Copy link
Contributor

@FlafyDev FlafyDev commented Dec 3, 2022

Describe your PR, what does it fix/add?

The PR fixes the if statement ConfigManager.cpp#L1039 which I assume should pass if one of the following conditions is true:

  1. needsLayoutRecalc is 1.
  2. the keyword (for example "general:gaps_out") contains "gaps_".
  3. the keyword starts with "dwindle".
  4. the keyword starts with "master".

right now it seems only the first if condition is corrent, while the others aren't.
The pr fixes the other conditions by

  1. changing VALUE to COMMAND
  2. fixing the second condition to currently check if the keyword contains "gaps_".

(for "hyprctl keyword general:gaps_out 120" it's: VALUE = "120" and COMMAND = "general:gaps_out".)

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

no

Is it ready for merging, or does it need work?

Yes

@vaxerski
Copy link
Member

vaxerski commented Dec 3, 2022

can you do .contains() instead of .find() != 0?

@FlafyDev
Copy link
Contributor Author

FlafyDev commented Dec 4, 2022

can you do .contains() instead of .find() != 0?

done (you mean instead of the .find() != std::string::npos, right?)

@vaxerski
Copy link
Member

vaxerski commented Dec 4, 2022

ya

@vaxerski vaxerski merged commit 686d6fc into hyprwm:main Dec 4, 2022
fufexan pushed a commit that referenced this pull request Dec 5, 2022
* fix: wrong layout recalculate if statement

* change from find to contains
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