(fix) WTrackTableView: assert track model, p prefix - #13623
Conversation
d9562ec to
655f22e
Compare
| TrackModel* pTrackModel = getTrackModel(); | ||
| if (!pTrackModel) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
trackmodel is unused here. Do we really need to check it then?
There was a problem hiding this comment.
Hm, not directly. But this is called by [Library], show_track_menu, so this is one function a controller could call before we have track model (or just item model).
Tbh I'm not entirely happy with this PR but it's the best / most simple fix I found.
There are two aspects:
- verify we have a selectionModel()
could happen there is none and we want to throw a warning then, eg. it's theoretically_ possible tosetSelectionModel(nullptr) - verify we have a model and simply return if there's none
that's the controller case I mentioned, and (again theoretically) it is possible such functions in other ways before the GUI is ready.
I think we should not throw an assertion then. Admittedly this only affects builds with DEBUG_ASSERTIONS_FATAL, but nevertheless such assertions are hard to debug.
There was a problem hiding this comment.
But I can remove it here, since it's in getSelectedRows(), too.
There was a problem hiding this comment.
yeah, remove it when getSelectedRows() already takes care of it.
| if (getTrackModel() == nullptr) { | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
I installed this as some sort of pre-DEBUG_ASSERT exit in case someone uses getSelectedRows() without verifying there's a model. (see my explanation above)
There was a problem hiding this comment.
yeah keep this one then. Though I'm undecided on whether it should be a debug assert as well...
655f22e to
6547e76
Compare
3 participants
followup for #13620
(sorry for the big diff)
I'll take care of merging to 2.5/main