Updated some old file filtering code. #6882
Conversation
I was trying to add a way to show/hide the .bak and other files with some filter buttons when I noticed the code in FileBrowser::addItems was copy pasta that had lava flowed from the much more modern Directory::addItems. In addition, only, FileBrowser::addItems was not respecting the filter's at all. I brought both of them into line with Qt 5 practices which now respects the m_filter list of extensions for both FileBrowser and Directory. In Directory::addItems I only needed to change where the match was being done, FileBrowser::addItems didn't try to filter/match at all.
He took a break from coding and left a note here: #6700 (comment). |
Ah, how does one contribute to a PR, let alone take it over? |
A few ways, but for this you can pull the PR and start from there on your own branch. |
If that's the case, I would like to start from here, I don't relish the idea of trying to bring all their work forward through 200+ commits, I will ofc pilfer some of the better ideas and patterns from it for the glory of Lommus. |
This is the way. |
DomClark
left a comment
DomClark
left a comment
There was a problem hiding this comment.
Looking much cleaner now, good job! Only a couple of comments.
Some fixes for const iterating the file list. Setting file name filters along with the call instead of seperately.
sakertooth
left a comment
sakertooth
left a comment
There was a problem hiding this comment.
LGTM, tested and working. Just some formatting suggestions.
Co-authored-by: saker <[email protected]>
5 participants
Summary
This is a bug fix for an unnoticed issue "File Browser filters do nothing" The built-in file extension filers are ignored on the top level leading to a messy projects directory.
Before, the File Browser would show any and all files in the projects directory, despite an existing filter in MainWindow.cpp "*.mmp *.mmpz *.xml *.mid" which FileBrowser was ignoring at the top level, but passed down to Directory, which used it correctly and in a modern way.
Background
I was trying to add a way to show/hide the .bak and other files with some filter buttons when I noticed the code in Directory::addItems was a modernized copy pasta of the older style (not using good Qt 5 conventions) FileBrowser::addItems.
In addition, only, FileBrowser::addItems was not respecting the filter's at all. I brought both of them into line with Qt 5 practices which now respects the m_filter list of extensions for both FileBrowser and Directory. In Directory::addItems I only needed to change where the match was being done, FileBrowser::addItems didn't try to filter/match at all.
Conflict
After doing all this I noticed that #6339 has been working on something similar (dare I say better!) to what I was going to do before I discovered this bug.
I would like to help out or take over that work so we can all have nice file browser filter buttons like Blender :)
@allejok96 hit me up!
I have checked out the current state of #6339 and it segfaults unfortunately; things have moved on a bunch (209 commits behind, woof!)
I am also willing to integrate #5772 and #6189 into this work if need be as these are all sore spots for me when using LMMS daily.