Fix csv rating export #4762
Fix csv rating export #4762
Conversation
src/library/basetracktablemodel.cpp
Outdated
| @@ -545,6 +545,16 @@ QVariant BaseTrackTableModel::roleValue( | |||
| } | |||
| switch (role) { | |||
| case Qt::ToolTipRole: | |||
| switch (field) { | |||
| case ColumnCache::COLUMN_LIBRARYTABLE_PREVIEW: | |||
There was a problem hiding this comment.
COLUMN_LIBRARYTABLE_PREVIEW is redundant, same as kDataExportRole.
Why do we need to skip COLUMN_LIBRARYTABLE_RATING and COLUMN_LIBRARYTABLE_TIMESPLAYED explicitly instead of simply displaying their value? Then this additional switch/case could be removed, same as before.
There was a problem hiding this comment.
I did consider the tool-tip on top of the rating stars and the played checkbox as visual clutter.
The use case of the tool-tip is to see the full string if the column is to small.
I have just noticed that this is also possible for these columns.
So I will re-enable the tool-tips.
OK?
There was a problem hiding this comment.
I have reverted these lines and git ammend
This allows to export/display all sort of valid TDRC formats
|
I have also fixed the year column display and exporting. |
src/library/parsercsv.cpp
Outdated
| ParserCsv::~ParserCsv() { | ||
| } // namespace | ||
|
|
||
| ParserCsv::ParserCsv() |
There was a problem hiding this comment.
Please replace this redundant definition with a = default; declaration (or no declaration at all if it is implicitly generated).
src/library/parsercsv.h
Outdated
| Q_OBJECT | ||
| public: | ||
| public: | ||
| ParserCsv(); |
There was a problem hiding this comment.
The default constructor declaration is probably redundant, see my comment. ~ParserCsv() override = default; would be more appropriate here.
|
Done. |
There was a problem hiding this comment.
Thank you! LGTM
I suppose it is ok not to preserve backwards compatibility, because this is just and internal format without any promises regarding its structure. It might break some users' use cases though, e.g. when exporting track lists into Excel for external play logs. Fixing this should be simple for them.
|
Thank you. I will take care merging this to main and resolve the conflicts. |
Some minor fixes for cvs export.