Skip to content

Log config options set by IPC #2714

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

Closed
wants to merge 5 commits into from
Closed

Conversation

killown
Copy link
Contributor

@killown killown commented Jul 7, 2025

fixes #2707

Message format:
[plugins/ipc-rules/ipc-utility-methods.hpp:287] Config options set via IPC will not update from /home/myuser/.config/wayfire.ini until Wayfire restart

{
LOGW("Config options set via IPC will not update from ", config_file_str,
" until Wayfire restart.");
warning_count++;
Copy link
Member

Choose a reason for hiding this comment

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

I would say we should ++warning_count every time, and when we hit MAX_WARNINGS, print a warning that we will suppress future warnings. Just so that users don't wonder why there are no subsequent warnings xd

@@ -23,6 +23,8 @@ class ipc_rules_utility_methods_t
private:
wlr_backend *headless_backend = NULL;
std::set<uint64_t> our_outputs;
int warning_count = 0;
Copy link
Member

Choose a reason for hiding this comment

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

We should make these static variables in set_config_options, this way it is clear that they are for that IPC method only.

@@ -281,6 +283,16 @@ class ipc_rules_utility_methods_t
return wf::ipc::json_error("Options must be an object!");
}

const char *config_file = std::getenv("WAYFIRE_CONFIG_FILE");
Copy link
Member

Choose a reason for hiding this comment

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

I think strictly speaking we don't really need to print the config file every time, we say this at the beginning of the log. So we could just say that we won't laod from the config file. Consider that for example config backends could also use a different config file altogether not the ini file.

@killown
Copy link
Contributor Author

killown commented Jul 7, 2025

Thanks for the suggestions. We should find a better way to explain set_config_options' implications without spamming logs like add comments.

@killown killown closed this Jul 7, 2025
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.

Provide a better way to let the user understand that setting options via IPC will make subsequent config file changes ineffective
2 participants