search

ActivityWatch / aw-core

Core library for ActivityWatch
Mozilla Public License 2.0
48 stars 46 forks source link

Only filter by app & title keys by default #100

Open iloveitaly opened 3 years ago

iloveitaly commented 3 years ago

Matching against url, editor payload keys, etc could be problematic. The existing rules are written against these two fields, as far as I can tell.

lgtm-com[bot] commented 3 years ago

This pull request introduces 1 alert when merging 4206a8d4cc571dd7fa1716fae6a93a57010b4e4f into b11fbe08a0405dec01380493f7b3261163cc6878 - view on LGTM.com

new alerts:

iloveitaly commented 3 years ago

@ErikBjare What do you think about updating the defaults of all existing rules with "select_keys":["app", "title"]? Agreed that it is more flexible to keep these defined on the frontend.

lgtm-com[bot] commented 3 years ago

This pull request introduces 1 alert when merging 876b32c5f747d8ab2bb408bb56318abef56051f1 into 5bd84b15759834e3e6e922db9bc39e5d1d4aea7a - view on LGTM.com

new alerts:

johan-bjareholt commented 3 years ago

What do you think about updating the defaults of all existing rules with "select_keys":["app", "title"]? Agreed that it is more flexible to keep these defined on the frontend.

If we added a way in the web-ui for the user to select the keys himself in a user-friendly way that would probably be OK for most rules. If it's a hidden configuration that the user can't change without exporting and then importing the rules again however I don't think it's a good idea to hide the behavior from the user.

iloveitaly commented 3 years ago

Agreed, my assumption here is we would add this to the webui at some point soon.

In the meantime, I'm wondering if it makes sense to add defaults to each default rule as I believe these are written to only match against the app & title keys. Introducing more keys may cause categorization errors.

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging cbdd7929306b1c9fa2c1cc8217acd67283f3e714 into 8aaa35376a4f0b270a1927dff4b4d34caee7707b - view on LGTM.com

new alerts:

iloveitaly commented 2 years ago

In the meantime, I'm wondering if it makes sense to add defaults to each default rule as I believe these are written to only match against the app & title keys. Introducing more keys may cause categorization errors.

@ErikBjare what do you think here?

ErikBjare commented 2 years ago

@iloveitaly Just keep the old behavior (server-side default to check all keys, undo https://github.com/ActivityWatch/aw-core/pull/100/commits/beac24bbd211cd333faec0ddb2cd20d0ea6e46b7), it's good enough for now.