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

Many advanced searches descrbed in the wiki do not seem to work #647

Open andrei-a-papou opened 5 months ago

andrei-a-papou commented 5 months ago

Some examples:

  1. Regex matching doesn't seem to work, example: /.*window.*/. This matches nothing, but I have four todos with the word window in them. If I just type window into the search field, I catch them all.
  2. Another example: window +sleek matches nothing, but I have three todos with the word window in them that have project +sleek assigned to them. window and +sleek doesn't show anything either.
  3. And here's a third: due: <= tomorrow+1d doesn't match todos that are both overdue and are due by the day after tomorrow. due: <= tomorrow works as expected.

There are more examples but perhaps it would be more efficient to deal with them iteratively or create an auto-test?

ransome1 commented 5 months ago

@zerodat if you can find some time, maybe you could check out these ones here.

zerodat commented 5 months ago

@andrei-a-papou there are actually a few different things going on here. First of all, there was a bug which prevented regex and string matches from working. That is fixed in PR #652 which I've merged into the filter query execution code. The old code was broken when sleek v2 changed the method of getting the text of a todo from todoObject.toString() to todoObject.string.

That solves your case 1, and also helps make case 2 work. But your query syntax in case 2 is actually a little wrong. To match a literal string in advanced query mode, you need to enclose the string in quotes. So for instance you should find that "window" and +sleek works with this fix. If you don't include the quotes, sleek will fall back to the simple string query, and in that case you would only match window +sleek if the word window came directly before +sleek in the text of the todo.

In your case 3, I think what is happening is that the necessary javascript function addIntervalToDate has been removed from sleek, although the call to it from the parser is still there. The import of it has been removed from the parser, but the call hasn't been. The addition operation on the date expression can't work without that function, which used to live in the recurrences.mjs file that no longer exists. @ransome1 could you explain where the old functionality from recurrences.mjs has gone? Is there something equivalent in the new sleek? If not, perhaps I can just copy the addIntervalToDate function into FilterQuery.js. I do wonder whether the recurrence math is being done properly now when a todo repeats, since that was formerly the primary purpose of addIntervalToDate.

ransome1 commented 5 months ago

In your case 3, I think what is happening is that the necessary javascript function addIntervalToDate has been removed from sleek, although the call to it from the parser is still there. The import of it has been removed from the parser, but the call hasn't been. The addition operation on the date expression can't work without that function, which used to live in the recurrences.mjs file that no longer exists. @ransome1 could you explain where the old functionality from recurrences.mjs has gone? Is there something equivalent in the new sleek? If not, perhaps I can just copy the addIntervalToDate function into FilterQuery.js. I do wonder whether the recurrence math is being done properly now when a todo repeats, since that was formerly the primary purpose of addIntervalToDate.

The whole module has been rewritten and lives now here: https://github.com/ransome1/sleek/blob/main/src/main/modules/ProcessDataRequest/CreateRecurringTodo.tsx. The addition of the interval value is done here: https://github.com/ransome1/sleek/blob/fcf9a4caf5e8310874ebf7b0f5565b308c89da34/src/main/modules/ProcessDataRequest/CreateRecurringTodo.tsx#L25C7-L25C26

The new recurrence implementation needed some iterations until it worked like defined in the wiki. It started off with some flaws in 2.x and has then been fine grinded over the course of this thread: https://github.com/ransome1/sleek/issues/611

Let me know if you find any indications, that the recurrences are not working as expected.

zerodat commented 5 months ago

Thank you, @ransome1! The new function CreateRecurringTodo/addRecurrenceToDate seems to replace the old function addIntervalToDate. I have fixed the filter query data expressions using that function.

andrei-a-papou commented 5 months ago

@zerodat Thanks for the explanation. I'll test things out when I get the chance. But I do think it's not very intuitive to have to quote a string for it to be treated correctly. I think users expect the software to just "do the right thing" here. All the more so when there's no match highlighting or any other kind of feedback when no match is found and you are left scratching your head as to why :)

ransome1 commented 5 months ago

The result of the current implementation can be tested in this pre-release: https://github.com/ransome1/sleek/releases/tag/v2.0.9-rc.1

andrei-a-papou commented 4 months ago

I'm using 2.0.12-rc1 and none of the test cases above seem to be working...

E.g. in my test file, I have four todos, only one of them contains the word blah (or character b). Yet the regex search /.*b.*/ matches all four todos.

image

zerodat commented 3 months ago

@andrei-a-papou would you show us the raw text that is in your test todo.txt file? It works correctly for me based on what you seemed to have in your screenshot, but matching for a single letter can give surprising results, for instance if you match a priority letter, a letter in a keyword field, and so on. Thanks,

-- zerodat

andrei-a-papou commented 3 months ago

Here you go:

blah blah 1 +project_1
2024-02-20 test window
2024-02-20 test regex +project_2
2024-02-20 another test
zerodat commented 3 months ago

Hmmm. Interesting. @andrei-a-papou when I test using that data and the pattern /.*b.*/ it works as it should: Screenshot from 2024-03-16 16-13-18

This is on Linux using the latest version of the sleek code.

amariusz commented 2 months ago

I'm using 2.0.13 on Linux and also observe bugs in searching. Using the sample data provided by @andrei-a-papou :

.*b.* returns everything 240506_183606_b

.*bl.* returns nothing

240506_183719_bl

also simple negation doesn't work like ! blah or NOT blah

240506_183754_not

zerodat commented 2 months ago

@amariusz something does seem to be broken in 2.0.13. I'll look into it a bit.

zerodat commented 2 months ago

@amariusz this is puzzling. When I download the sleek-2.0.13.AppImage, it has the buggy behavior that you described. But if I build the same version from the github repo, it seems to work correctly. You can try it yourself on linux, like this:

git clone https://github.com/ransome1/sleek.git
cd sleek
yarn
yarn run package
chmod +x release/build/sleek-2.0.13.AppImage
release/build/sleek-2.0.13.AppImage

That will build you an new AppImage from the source and execute it. When I did this, it did the searches correctly. Perhaps something went wrong in the creation of the official releases. I'm curious to hear whether it will work when you try it.

amariusz commented 2 months ago

@zerodat thanks for the info and instructions. I've followed them and have the same results. I've tested /.*b.*/, /.*bl.*/, NOT /.*bl.*/ and NOT "blah" - all have worked correctly.

240507_070444_local_build

(BTW I realized that quotes are also necessary for NOT to work, but on official build it wasn't working anyway)

zerodat commented 2 months ago

@amariusz thanks for testing and confirming that! I think next we need help from @ransome1 to diagnose why the official builds are not working properly. I can't reproduce the problem on a fresh build, so I can't debug it. I wonder if it is possible that the official build is pulling in an old or stale version of the FilterLang.js file (rather than regenerating from the FilterLang.pegjs source code)?

amariusz commented 2 months ago

I have no idea but apart from buggy behaviour that's also a potential security issue to investigate. I agree @ransome1 is right person to look into that :)

ransome1 commented 2 months ago

@amariusz @zerodat I'm afraid I can't be of big help at the moment due to private reasons.

I'm not sure if I changed anything in regards of the search. I share the feeling Zerodat has, that it might have something to do with the pegjs hasn't been regenerated properly.

Can I ask you to queck the latest pre-release real quick. And let me know if this happens there as well? Since I havn't received any bug reports related to it, I can push it to production and release it rather soon.

amariusz commented 2 months ago

Searching in sleek-2.0.14-rc.1.AppImage behaves like official 2.0.13. No improvement here.