Fuzzy text search, part 2: Add fuzzy search to learn page - #197
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds a Qt playlist list model with playlist roles and duration formatting. Merge Risk: ⚪ Minimal · up to The PR is merge-ready after normal checks and review; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/learn/qml/Muse/Learn/internal/playlistmodel.cpp`:
- Around line 48-50: Update PlaylistModel::rowCount to return zero when the
supplied parent QModelIndex is valid, while preserving m_playlist.size() for the
invalid root index used by this flat list model.
In `@framework/learn/qml/Muse/Learn/LearnPage.qml`:
- Around line 204-222: Update the LearnPage playlist flow around the
SortFilterProxyModel to pass whether a search is active into Playlist.qml, then
handle the zero-row state separately from load failure. When an active fuzzy
search has no matches, display a dedicated no-results message; preserve the
existing “unable to load these videos” message for genuine playlist loading
failures.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ea3f57e1-6b74-42e3-8de8-02a6d6206ee8
📒 Files selected for processing (7)
framework/learn/qml/Muse/Learn/CMakeLists.txtframework/learn/qml/Muse/Learn/LearnPage.qmlframework/learn/qml/Muse/Learn/internal/Playlist.qmlframework/learn/qml/Muse/Learn/internal/playlistmodel.cppframework/learn/qml/Muse/Learn/internal/playlistmodel.hframework/learn/qml/Muse/Learn/learnpagemodel.cppframework/learn/qml/Muse/Learn/learnpagemodel.h
5ff1a47 to
4833259
Compare
|
/build |
|
Build dispatched: https://github.com/musescore/muse_framework/actions/runs/32007229865 |
4833259 to
245d2f4
Compare
|
/build |
|
Build dispatched: https://github.com/musescore/muse_framework/actions/runs/32268199111 |
Refactors the playlist models to a QAbstractListModel subclass to allow the use of our SortFilterProxyModel.
245d2f4 to
4c42b96
Compare
|
@Eism I've rebased again to fix the broken build |
|
/build |
|
Build dispatched: https://github.com/musescore/muse_framework/actions/runs/32358870039 |
|
Tested fuzzy search on Home>Learn page on Win10, Mac13.7.8, LinuxUbuntu26.04 LTS. It seems to better than in main (e.g. 'dnamics' points to videos with 'dynamics' text) @avvvvve You can try it on your side too |
Part of: musescore/MuseScore#15983
Refactors the playlist models to a QAbstractListModel subclass to allow the use of our SortFilterProxyModel.