Skip to content
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

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

Open
andrei-a-papou opened this issue Jan 26, 2024 · 18 comments
Open
Labels
bug Something isn't working

Comments

@andrei-a-papou
Copy link

andrei-a-papou commented Jan 26, 2024

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
Copy link
Owner

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

zerodat added a commit to zerodat/sleek that referenced this issue Jan 28, 2024
zerodat added a commit that referenced this issue Jan 28, 2024
solves filter bug due to todoObject.toString, issue #647
@zerodat
Copy link
Collaborator

zerodat commented Jan 28, 2024

@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
Copy link
Owner

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: #611

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

@zerodat
Copy link
Collaborator

zerodat commented Jan 29, 2024

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.

zerodat added a commit that referenced this issue Jan 29, 2024
@andrei-a-papou
Copy link
Author

@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
Copy link
Owner

ransome1 commented Feb 3, 2024

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
Copy link
Author

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
Copy link
Collaborator

zerodat commented Mar 16, 2024

@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
Copy link
Author

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
Copy link
Collaborator

zerodat commented Mar 16, 2024

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
Copy link
Collaborator

amariusz commented May 6, 2024

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
Copy link
Collaborator

zerodat commented May 7, 2024

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

@zerodat
Copy link
Collaborator

zerodat commented May 7, 2024

@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
Copy link
Collaborator

amariusz commented May 7, 2024

@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
Copy link
Collaborator

zerodat commented May 7, 2024

@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
Copy link
Collaborator

amariusz commented May 7, 2024

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
Copy link
Owner

ransome1 commented May 7, 2024

@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
Copy link
Collaborator

amariusz commented May 7, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 2.0.13
Development

No branches or pull requests

4 participants