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.
yeah or like .../parent_directory/file_name.wav |
|
My bad, I just now see that @AW1534 is talking about the context menu filepath line and not changing the browser item text. I'm all for such a change, maybe even to have it be just the name of the file, without the directory.
More reason for it to just be the filename, I'd think. Sorry for the confusion, @AW1534 and @sakertooth 🙏 |
|
LGTM! 👍 |
|
@AW1534 I believe the issue with item context menus is also resolved?
If you'd still like to give the user the ability to find his file using LMMS exclusively, maybe @sakertooth could implement a context menu option called "Open in browser tree" which would just find the file in the LMMS browser used for the search? That would probably warrant a second PR. Also, @sakertooth, how difficult would it be to add this search behavior into the effect broswer? |
This PR revamps the search functionality within the file browser, with the intent of improving the code and user experience.
Changes:
FileBrowser::onSearch. As such,FileSearchwas removed. This should make the search functionality more easy to follow.