-
Notifications
You must be signed in to change notification settings - Fork 46
Add IPC method to select a profile #108
base: master
Are you sure you want to change the base?
Conversation
Harmless, because it was using size of a pointer, which will always be the same, but still confusing to read. Using `sizeof *matches` makes it so any change to the type of `matches` will keep the expression correct.
makes |
Ok, I think my latest commit fixes it. I left a few questions in the commit message about the implementation. |
The compositor will broadcast events followed by a What I originally thought of was that the profile names would act as a tie-breaker. Setting a profile name would add a new condition to if (state->selected_profile_name != NULL && profile->name != NULL &&
strcmp(state->selected_profile_name, profile->name) != 0) {
return false;
} So that the normal matching logic still applies, ie.
To implement this, the "currently selected profile name" would need to be saved in I'm open to other ideas though. |
So basically you would just have me update the current name, then call Would you prefer |
Yup!
I'd prefer to pick a subcommand name that describes an action. |
This allows us to add an extra constraint for try_apply_profiles, which now takes the selected profile name into account.
I wonder if current and pending names wouldn't be necessary? Otherwise using a name that doesn't exist would break any further transitions. And using |
Maybe we should try and report an error back to |
Sounds sensible. Also adding something like |
Rework how varlink_connection_call is called so the logic is used by all methods. Uses method_name as a sign that argv[1] was matched by some known method.
Would have to forbid profiles named But I like the idea. |
@@ -425,6 +429,10 @@ static bool try_apply_profiles(struct kanshi_state *state) { | |||
assert(wl_list_length(&state->heads) <= HEADS_MAX); | |||
// matches[i] gives the kanshi_profile_output for the i-th head | |||
struct kanshi_profile_output *matches[HEADS_MAX]; | |||
if (state->current_profile && match_profile(state, state->current_profile, matches)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I don't understand what this does
@@ -36,7 +36,12 @@ static bool match_profile(struct kanshi_state *state, | |||
return false; | |||
} | |||
|
|||
memset(matches, 0, HEADS_MAX * sizeof(struct kanshi_head *)); | |||
memset(matches, 0, HEADS_MAX * sizeof *matches); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: missing parentheses after sizeof
So, I've been dog fooding another version of this for a while now, which implements the idea in #108 (comment) I am, however, hitting an UX issue because of it. My config is:
I start with I didn't like this experience, and I think I prefer the style where That's what the code you asked about in #108 (review) implemented: without that, selecting a profile led to kanshi moving to it, then getting a wayland callback, and moving back to the first match in the config (as described in #108 (comment)); with that, it makes sure not to override a manually selected profile (which was set via the |
As discussed on IRC, two potential solutions:
|
This "works", in the sense that the server receives the request and applies the profile. However, it seems that it immediately triggers a "detected change" event, and the server applies the old profile again:
I'm looking into the code to try and understand how to make this not happen, but would definitely appreciate tips on where to look :)
I hope my approach is correct, too.