-
-
Notifications
You must be signed in to change notification settings - Fork 112
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
what is the actual point of per-room pushrules? #651
Comments
and while we're at it: per-sender and per-content pushrules are just special cases of underride rules, and per-sender pushrules are barely used. |
The obvious way to extend this is to (a) allow clients to specify additional |
(otoh, maybe underride rules are fine) |
I have limited insight into the real-world applications of this API, but as far as I can tell, the entire API can be generalized as such without losing any of its current capabilities, and while resolving the awkward bits: [snip -- removed and updated after discussion, see update below] I've already been "test-driving" this generalized model by internally converting push rules to it, in my protocol implementation. I have not run into any issues with it, and it seems to be capable of representing the entire class of current push rule configurations, but I don't know if there are eg. server-side implementation concerns that I'm overlooking here. Of course, the actual representation towards the client would be a breaking protocol change. (Bonus: in this generalized model, device-specific rules - which #273 states are eventually a desirable feature - can be represented simply by having an |
After discussing with @richvdh, it turns out that the spec on this is a bit misleading, and my interpretation of it incorrect; single-match semantics apply across all rules of any kind, not "within the same kind". Therefore, the general model becomes much simpler: The push rule configuration is simply a list of rules, where each rule has zero or more conditions. When matching rules, the first match terminates the matching process, ie. only the first match within the entire list of rules will be considered. The current model would map to this generalized model as such: each current 'kind' of rules (override, room, underride, etc.) would map to its own list of rules. These lists then get concatenated into one final rule list, in the currently-specced order. This provides a migration path. However, arbitrary extra rules can be added after this change in any position in the list, and there are no restrictions on what sorts of conditions any of the rules can contain. Room and sender matching would need to be converted to generic conditions, for this to work. |
@joepie91 I think the main reason to have some sort of structure within pushrules is to help clients map the pushrules back into UI. Most people don't want to have to manually maintain a list of rules: they just want a few options they can turn on and off. If the rules are a completely unstructured list, then it becomes very difficult for clients to map back and forth between UI and rules. |
This doesn't really achieve that, though; you can already specify arbitrary rules through overrides and underrides, so clients need to already be either capable of processing/displaying arbitrary rules, or be willing to ignore them. Generalizing the model further wouldn't change that. Of course clients do need to recognize the rules that they themselves have set. But is that not what the rule IDs are for? |
Also, to be clear, I'm definitely not arguing that my generalized model is the best possible model for push rules that we could have (and it probably isn't). I'm only arguing that it's a conceptually-compatible model to the current one that would solve a bunch of its complexity and not make anything worse in the process :) |
since you can only have one per-room pushrule, if you want to do anything more intelligent than 'treat all events in this room the same', you end up using overrides or underrides anyway
The text was updated successfully, but these errors were encountered: