Improve search behavior in the file browser - #7679
Conversation
|
For some reason while searching, there is a segfault when closing the program. Doesn't seem to be a concern, but unsure why this happens. |
Reducing the search space for performance is no longer a problem now, so there's no harm in allowing these directories to be considered (and we can avoid potential problems with how files are organized on users machines). Making sure to always check agaisnt the blacklist was also a bit error-prone since its possible to forget to check against it (like with how searching was implemented here, but it did not seem to matter at that point).
Most notable differences are that we cancel the search automatically in the destructor when closing the application, and we use a mutex to prevent ordering conflicts when beginning and ending searches.
Does this mean I can no longer, for example, type "mid" and see all my midis? this is something I do often and I think this behaviour should be preserved. |
No, it's just that the search is no longer limited by files with certain extensions. Instead, the search now considers all files regardless of its extension. If you type "mid", it will treat it as a token and find any file that has it in its name. ".mid" will probably find all midis for you. I did this because if a file was named "foo.medi" but was a perfectly working midi file, then the search wouldn't find it if it considered extensions regardless if the search filter was for e.g. ".medi". |
Since we always cancel the search if one is already running before starting up a new one, theres no need for mutual exclsuion since the cancellation logic should provide that.
We shouldn't update the search indicator directly in the onSearch function because those updates may get out of order with the updates that are queued by the auxiliary thread, potentially resulting in inconsistent state.
sakertooth
changed the title
|
Is this ready for testing? |
Feel free. |
|
Hell yeah! Merge ASAP. 🚀 |
|
Searching looks good but im not a big fan of the file path being in the context menu as these can get long Edit:maybe change it to only show the parent dir instead of the whole canonical file path? |
|
We could use ellipsis, but yeah I guess I can remove it. |
Something like |
This reverts commit 98b73e9.
|
Okay, new double quote feature added and bugs fixed, it should be fine now. Any testing will be appreciated 👍 |
|
|
|
Gonna retest! |
|
Anything blocking this from being merged? |
Other than the merge conflicts, I think @bratpeki mentioned a bug with this PR in the Discord that I haven't got around to fixing yet. |
This reverts commit b2ad4ea.
|
Hello, quick update: I don't think I have any remaining things to do for this PR. If anyone wants to revisit this/do some final testing, I appreciate it. In any case, I think this is ready for merge. |
tresf
left a comment
tresf
left a comment
There was a problem hiding this comment.
Tested, couldn't find anything obviously wrong with it.
This does get a bit taxing for things like Volumes and My Home especially if you have mounted network volumes or additional drives, but if that becomes an issue, we can file an enhancement later.
Thanks for the hard work on this @sakertooth!
|
Will get back to this soon @messmerd. To any maintainers, just don't merge this right now. |
I agree. It is tasking performance wise, I experienced this as well at certain times. I think ideally we wouldn't be searching throughout the entire home directory but instead use more specific paths. We could probably also remove Home and Root because we have the "Favorites" section now thanks to work done by @AW1534 (but this might cause backlash so not sure if we want to pursue it in the future). |
6 participants
This PR changes the search functionality within the file browser, with the intent of improving the code and user experience.
Changes: