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

Replace monitor rule when disabling head. Closes #5978 #6136

Merged
merged 6 commits into from
May 23, 2024

Conversation

levnikmyskin
Copy link
Contributor

Describe your PR, what does it fix/add?

Fixes Issue #5978. The main reason was that the monitor rule was not updated, and thus the head would get re-enabled when calling performMonitorReload

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

I'm not sure this is the best way of doing it. Would it be a better idea to make Monitor.activeMonitorRule a pointer to the actual rule?

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

See above.

@levnikmyskin
Copy link
Contributor Author

levnikmyskin commented May 19, 2024

As mentioned in the issue thread, it looks like with this I can disable monitor via kanshi, but cannot re-enable it. Probably because of a similar problem. I'll try to fix that too

UPDATE: looks like it's working after last commit. I'll wait for people to confirm this in the issue thread before re-requesting review

@vaxerski
Copy link
Member

tag me when ready and tested

@levnikmyskin
Copy link
Contributor Author

@vaxerski this is ready to be reviewed

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

rest ok, I assume tested and fixes the issue?

src/protocols/OutputManagement.cpp Outdated Show resolved Hide resolved
@levnikmyskin
Copy link
Contributor Author

rest ok, I assume tested and fixes the issue?

Yes, a few people reported back and it looks like we can close the issue

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

thanks!

@vaxerski vaxerski merged commit eea0a6a into hyprwm:main May 23, 2024
10 checks passed
@levnikmyskin levnikmyskin deleted the fix/update_monitor_rule_issue#5978 branch May 23, 2024 19:23
The-Briel-Deal pushed a commit to The-Briel-Deal/Hyprland that referenced this pull request May 25, 2024
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