-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
[feature request] *partial match* on *full path* #169
Comments
This seems reasonable. The search code is here, and would need to be modified to match on parts. Note this gets called a lot, so performance optimization is key. I'm marking this as a good first bug in case anyone wants to pick it up. quickmove-extension/src/popup/baseItemList.js Lines 483 to 495 in e5490f3
|
Could it be something like this? It checks if any word in the search strings matches any part of the folder path. |
Maybe this if (!hasAccent) {
itemText = itemText.normalize("NFD").replace(DIACRITICS, "");
} has to be applied also to the path before spitting in segments? |
Perhaps it would be better to enable this new behavior as an option? |
Something like that might work. You might need to instead traverse the folder nodes to root instead of splitting by What I am worried about is performance though. This makes the lookup O(n^2), and it is done for all folders. Ideally we'd build some sort of search tree or full text indexing, though I know that is a bit more work. Do you see a way to optimize performance here? |
Do you think that could be a problem? The folder matching should still occur in this case. Also, what does the '/' in the folder name represent?
Given that we are handling folders and not messages, do you think the average user will notice any impact?
Maybe in this line https://github.com/micz/quickmove-extension/blob/d7fe72e1b9c3f90b22d8abacb55d4991bee80b02/src/popup/baseItemList.js#L528 the use of |
Yes, this has been an issue, there was an Owl issue here recently. Replacement is simple though see
It might also make sense to have an Iterator version of fullNameParts so that walking stops when a segment is found.
I've seen a few reports saying that generating the folder list is very slow, so I do believe this is the case. Maybe when there are many many many folders on a slow machine. If we make it a default-off pref then I think we could risk it and wait until someone complains that it is slow when they turn it on. I don't know off hand if it would be faster, but we could also maybe cache a string in the folder nodes lazily. Each node has a full text string that includes its own name and the name of the parents, so when we want to check if a folder should be in the list we just need to run one |
I was thinking exactly about that.
That's a great idea. Maybe there is no need to split the full path string either. |
I modified the code: micz@80d41af There is no need to split the path. I tested it with 5000 folder and there is no delay. Let me know if it's ok for you and how you want to procede. |
Sweet, thanks for testing, this is good news. I'm a bit reluctant to use the path though for matching. I believe there are some edge cases where the folder path does not match the display name. I was certain I had an example from a past issue but couldn't find it. In theory though, the display name can differ from the folder path. One potential case is when the folder name is localized, e.g. the path is I'm not 100% certain this is true, it might be worth asking in the add-on developer's channel if it is safe to use the path to match folder display names or if my suspicion is true. The safe path is to actually use the name and iterate upwards. |
I understand your point, you're right. I'll try to use the lazy loading you suggested to create a string iterating upwards using the display names. |
Yep that works for me, thank you so much for persisting on this! |
I added a new property to FolderNode, #_fullSearchString, and then implemented a getter, with lazy laoding of the property. In the test with 5000 folders, it is as fast as before. I hope I got it right this time! |
If that #169 (comment) is right, the next steps could be:
In step 3, do you prefer a single loop with an |
Hi,
Thanks for the very nice tool. I used Nostalgy++ for a while, but I prefer your tool for the beautiful GUI and its ability to handle correctly custom key bindings.
However, there a thing that I miss a lot from Notalgy++, is the ability to match full path of directories and even with partial match.
e.g. I'm a teacher. There are several courses I teach every year. I teach courses with similar titles to various audiences. So I have a directory structure similar to: <year_number>/<course_audience>/ ; example : 2024-2025/Master/job.
When I select a target directory for batch moving emails, I would like to be able to type "25 ma jo" and get this examples directory selected over all the other ones.
The text was updated successfully, but these errors were encountered: