-
Notifications
You must be signed in to change notification settings - Fork 17
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
basic implementation for exclude filter #41
Conversation
@mhelleboid we are still searching for a new maintainer but as you familiar (a bit) with the codebase of the plug-in would are ok with checking the other PRs. |
@PackElend, yes sure, I can have a look at opened PRs. |
As suggested by @PackElend , I am here to help review the code. I have confirmed that the code works as described, and the test case has passed. When moving a note from a column with the "tag: -wip" label, it will set the "wip" flag. I am a bit confused at the first glance, but it should be a reasonable behavior. The biggest impact is the change in the column filtering rule, which has been changed from "some" to "all." I think it makes sense, but it may cause a weird issue for the user. To replicate the issue:
Furthermore, I found a minor defect (which will probably be ignored by the user and no need to fix?):
|
Thanks @benlau for the review, I will take your feedback in consideration and update this PR! |
@mhelleboid how it is going with the update ? |
@PackElend I will try to work on it next weekend |
Yes, I think this is an acceptable behavior, removing this behavior leads to others less acceptable behabior |
Maybe this should be configurable, but I like the new behavior where the filtering rule actually filters notes |
I cannot reproduce it, @benlau can you give me a more detailed scenario please? |
@mhelleboid I think it is ok. We may not need to fix it.
sorry, a bit busy for this week, what if we just ignore this issue as it is a very minor issue. |
@mhelleboid @benlau it looks like your peer review works quite well. @mhelleboid do I find you n the forum? |
I can try
Just created an account with the same username |
Exclude filter
Implementation of exclude filter, which allows me to easily use "work in progress" column for todo with the following configuration :
May be a solution to #29
All Tags
Also added an "alltags" rule to ensure that all tags are present or not present, which allows for example to add a "Priority" column with the following configuration
New Behavior
Change the behaviour of "completed: false" and "completed: true" statement when used with tag/tags statements.
And may have other impacts, like for example the facts that column tag/tags and filters tag/tags must all be satisfied.
See the test modification for the behaviour change
Next
I can try to write more tests if needed