Skip to content

feat: Add config reload delay option and refactor reload logic #2715

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

Merged
merged 1 commit into from
Jul 7, 2025

Conversation

ammen99
Copy link
Member

@ammen99 ammen99 commented Jul 7, 2025

Fixes #2493 and potentially other long standing issues related to config reload.

An open question is whether we want to set the default to something like 50 or leave it to 0 to keep the old behavior. Thoughts?

@ammen99 ammen99 force-pushed the delay-config-reload branch from 8b4ffc2 to d8fbaad Compare July 7, 2025 14:30
@soreau
Copy link
Member

soreau commented Jul 7, 2025

An open question is whether we want to set the default to something like 50 or leave it to 0 to keep the old behavior. Thoughts?

My opinion is that we should keep old behavior here and set the default to 0. For example with wcm, anything other than 0 would break i.e. scrolling on a spinner to change an int or float value and see the result in real-time.

The thing I would like to see fixed in all this is, starting wayfire with something like wayfire -c /tmp/wayfire.ini where /tmp/wayfire.ini does not exist, then while wayfire is running, creating that config file and changing it would work without having to restart wayfire.

@ammen99
Copy link
Member Author

ammen99 commented Jul 7, 2025

An open question is whether we want to set the default to something like 50 or leave it to 0 to keep the old behavior. Thoughts?

My opinion is that we should keep old behavior here and set the default to 0. For example with wcm, anything other than 0 would break i.e. scrolling on a spinner to change an int or float value and see the result in real-time.

The thing I would like to see fixed in all this is, starting wayfire with something like wayfire -c /tmp/wayfire.ini where /tmp/wayfire.ini does not exist, then while wayfire is running, creating that config file and changing it would work without having to restart wayfire.

Did we not fix this a long time ago? There is code for it in the default config backend at least.

@ammen99
Copy link
Member Author

ammen99 commented Jul 7, 2025

My opinion is that we should keep old behavior here and set the default to 0. For example with wcm, anything other than 0 would break i.e. scrolling on a spinner to change an int or float value and see the result in real-time.

Good point, I will set it to 0 by default.

@soreau
Copy link
Member

soreau commented Jul 7, 2025

The thing I would like to see fixed in all this is, starting wayfire with something like wayfire -c /tmp/wayfire.ini where /tmp/wayfire.ini does not exist, then while wayfire is running, creating that config file and changing it would work without having to restart wayfire.

Did we not fix this a long time ago? There is code for it in the default config backend at least.

Hm, I'm not sure if it works or not, would have to test it.

@killown
Copy link
Contributor

killown commented Jul 7, 2025

Fixes #2493 and potentially other long standing issues related to config reload.

An open question is whether we want to set the default to something like 50 or leave it to 0 to keep the old behavior. Thoughts?

you pr is amazing, fixed the issue!

@killown
Copy link
Contributor

killown commented Jul 7, 2025

WCM is working with 50ms.

This PR doesn't just fix the IPC issue, it also fix the outputs being disabled due to the config reload issue. This no longer happens.

I believe 50ms should be the default option, as it will prevent many issues, this pr is bigger than you imagine, besides the simple code.

@ammen99 ammen99 force-pushed the delay-config-reload branch from d8fbaad to 81c9260 Compare July 7, 2025 21:51
@ammen99 ammen99 merged commit 12ba346 into master Jul 7, 2025
8 checks passed
@ammen99 ammen99 deleted the delay-config-reload branch July 7, 2025 22:06
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.

IPC: Saving modifications in wayfire.ini causes watch_events() to stop working
3 participants