Fix empty library table after start #3935
Conversation
|
So this fixes both Can't test since I can't reproduce (with 2.3). |
|
Did you test both with a clean install and an upgrade from 2.2.x? |
|
Yes, here is the result after a update from a clean 2.2.3 install |
|
Who is a good person to review this? I am not up to speed with this issue |
src/widget/wtracktableview.cpp
Outdated
| } | ||
|
|
||
| //saving current vertical bar position |
There was a problem hiding this comment.
Please put spaces between // and words:
// Saving current vertical bar position
// using the address of the track model as key.
src/widget/wtracktableview.cpp
Outdated
| sortColumn++; | ||
| // sort, find the first valid sort column and sort by that. | ||
| int sortColumnIndex = -1; | ||
| constexpr int kMaxPobeIndex = 10; // just to avoid an endless while loop |
There was a problem hiding this comment.
what does "Pobe" mean? Instead of a magic number can you use the number of valid columns?
There was a problem hiding this comment.
This is unknown here and there is no function to access this number.
The original code silently assumes that there is a valid column among the first few.
This is a safety net together with the ASSERT below.
|
Done |
|
I just tested this and this solves the issue for me. |
src/widget/wtracktableview.cpp
Outdated
| } | ||
| DEBUG_ASSERT(sortColumn != TrackModel::SortColumnId::Invalid); |
There was a problem hiding this comment.
Using a debug assert is wrong, because it will trigger when reaching kMaxProbeIndex, which is possible.
Why do you need an artificial constant instead of using model()->columnCount() as the upper limit?
|
Thank you for the hint. Fixed. |
|
@uklotzde Any chance this also fixes First activation of external playlist: Empty view for you? |
src/widget/wtracktableview.cpp
Outdated
| while (sortColumn < 0 || trackModel->isColumnInternal(sortColumn)) { | ||
| sortColumn++; | ||
| // sort, find the first valid sort column and sort by that. | ||
| int sortColumnIndex = -1; |
There was a problem hiding this comment.
The idiomatic implementation would be a simple for loop with a break statement.
|
Empty track table view of external playlist is fixed. I cannot reproduce the other issues, even when switching languages. |
The issue was an invalid stored sort column, that prevents the initial select() call.
Now it falls back to either a default or the first column.