ransome1 / sleek

todo.txt manager for Linux, Windows and MacOS, free and open-source (FOSS)
https://github.com/ransome1/sleek/wiki
MIT License
1.28k stars 99 forks source link

Cannot match all projects or contexts using + or @ #616

Closed jloow closed 5 months ago

jloow commented 6 months ago

Bug Report

App Version: 2.0.3

Platform: macOS

Installation Method: Direct Download

Bug Description: Using advanced search, I cannot filter my todo for + or @ (which is supposed to match any todo with a project or context). It is possible to match other projects or context using +test or @test. Specifically I would like to filter for any todos without a project (i.e. "!+" or "not +") or without a context (i.e. "!@" or "not @")

Steps to Reproduce:

  1. Enter, e.g., "+", "!+", "not +", "@", "!@" or "not @" without quotations marks in the search field.

Expected Behavior: To find todos that don't have any project or that don't have any context.

Actual Behavior: When using NOT operators, no todos are displayed. Otherwise, all todos are displayed.

Additional Information: The same bug is present in 2.0.2 installed through homebrew.

jloow commented 6 months ago

I also tested this on Linux, and it seems to be an issue there as well.

ransome1 commented 6 months ago

@jloow can you please share an example todo list, the exact search string you're applying and the expected result?

jloow commented 6 months ago

Sure!

Example todo.txt:

2023-12-13 Test 1 +project_a
2023-12-13 Test 2 +project_b
2023-12-14 Test 3 @context_a
2023-12-14 Test 4 @context_b
2023-12-15 Test 5 +project_a @context_a
2023-12-15 Test 6

Search string: +

Expectation:

2023-12-13 Test 1 +project_a
2023-12-13 Test 2 +project_b
2023-12-15 Test 5 +project_a @context_a

Result:

2023-12-13 Test 1 +project_a
2023-12-13 Test 2 +project_b
2023-12-14 Test 3 @context_a
2023-12-14 Test 4 @context_b
2023-12-15 Test 5 +project_a @context_a
2023-12-15 Test 6

Search string: @

Expectation:

2023-12-14 Test 3 @context_a
2023-12-14 Test 4 @context_b
2023-12-15 Test 5 +project_a @context_a

Result:

2023-12-13 Test 1 +project_a
2023-12-13 Test 2 +project_b
2023-12-14 Test 3 @context_a
2023-12-14 Test 4 @context_b
2023-12-15 Test 5 +project_a @context_a
2023-12-15 Test 6

Search string: !+ or not +

Expectation:

2023-12-14 Test 3 @context_a
2023-12-14 Test 4 @context_b
2023-12-15 Test 6

Results: None.


Search string: !@ or not @

Expectation:

2023-12-13 Test 1 +project_a
2023-12-13 Test 2 +project_b
2023-12-15 Test 6

Result: None.

ransome1 commented 5 months ago

@jloow this does not solve the issue at hand – which I have to admin, I have not looked into yet – but since you are using the advanced search, you might be interested in a proof of concept, that has been implemented based off this feature request: https://github.com/ransome1/sleek/issues/179

Feel free to test it out and share your feedback.

ransome1 commented 5 months ago

@zerodat are you around these days? @jloow report is valid. The Wiki says @ or + should narrow down the list to contexts or projects.

But doing so has no effect.

I also looked into the implementation but could not figure out exactly how it works. The easiest would probably be, if you could take a look at it.

At some point, we should add some test cases, since this is easy to test.

zerodat commented 5 months ago

@ransome1, I'm still out here. :smile: I'll try to take a look at this issue.

zerodat commented 5 months ago

@ransome1 still taking a preliminary look and I will be busy for the next 8 or 10 hours, but later I will look deeper. The code for the query engine seems basically the same as it was back in the 1.2.9 version. I confirmed that it works properly in that version. What should be happening is that in the case of a query like "@" or "+" it will check the state of todoObject.contexts (line 127 of FilterQuery.js) or todoObject.projects (line 113). That all looks like it ought to work unless the todoObject is much different now in version 2 of sleek. I'll try to do some debugging later to see where it is going wrong.

zerodat commented 5 months ago

@ransome1, I confirmed that this worked in 1.2.9 and no longer works in 2.0.6.

The problem seems to be that I tested for the list of projects or contexts to be empty by just testing todoObject.contexts (or projects) as a boolean value. That works if the contexts is null or undefined when there are none. However, in Javascript an empty list tests as "true" (which is the opposite of the behavior in Python, my usual language).

It seems like the 2.0.6 version is setting contexts or projects to an empty list rather than undefined in the case where there are no contexts or projects. Maybe that's a change in behavior of the upstream todo parser.

In any case, we need to change the code in FilterQuery.js to test the length of the list explicitly. I'm submitting a pull request that fixes the issue.

ransome1 commented 5 months ago

@jloow I can confirm that @zerodat PR fixed this. I will include it into the next release.

jloow commented 5 months ago

Great! I've been a bit busy but I'll be sure to check out the next release.

ransome1 commented 5 months ago

@jloow @zerodat this has been released with 2.0.7: https://github.com/ransome1/sleek/releases/tag/v2.0.7

jloow commented 5 months ago

I've done some quick testing and this seems to have been fixed. The other updates work really well as well.

How does this work - do I close the issue?

ransome1 commented 5 months ago

How does this work - do I close the issue?

Yes please :)