From c7ab0c5a82bdc55748618811aba9402ef30bb1d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 28 Nov 2021 18:42:10 +0100 Subject: [PATCH 01/21] Fix m3u list parsing with /r/n end of line This fixes a regression form https://github.com/mixxxdj/mixxx/commit/a006552fec6877facc4aaa22080cc67d6699941d --- src/library/parserm3u.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index d07c2a326e1..3af052b0974 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -70,13 +70,6 @@ QList ParserM3u::parse(const QString& filename) { // We replace any '\r' with '\n' if applicaple // This ensures that playlists from iTunes on OS X can be parsed QByteArray byteArray = file.readAll(); - //detect encoding - bool isCRLF_encoded = byteArray.contains("\r\n"); - bool isCR_encoded = byteArray.contains("\r"); - if (isCR_encoded && !isCRLF_encoded) { - byteArray.replace('\r', '\n'); - } - QString fileContents; if (isUtf8(byteArray.constData())) { fileContents = QString::fromUtf8(byteArray); @@ -87,7 +80,7 @@ QList ParserM3u::parse(const QString& filename) { } QFileInfo fileInfo(filename); - const QStringList fileLines = fileContents.split('\n'); + const QStringList fileLines = fileContents.split(QRegExp("\r\n|\r|\n")); for (const QString& line : fileLines) { auto trackFile = playlistEntryToFileInfo(line, fileInfo.canonicalPath()); if (trackFile.checkFileExists()) { From 52a86b81073d622cf9b82214dd50ddb68109ba83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 28 Nov 2021 22:38:27 +0100 Subject: [PATCH 02/21] Move end of line regex to the global namespace --- src/library/parserm3u.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index 3af052b0974..af5a9373b9e 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -25,6 +26,8 @@ namespace { // according to http://en.wikipedia.org/wiki/M3U the default encoding of m3u is Windows-1252 // see also http://tools.ietf.org/html/draft-pantos-http-live-streaming-07 const char* kStandardM3uTextEncoding = "Windows-1250"; +// Note: The RegEx pattern is compiled, when first used the first time +const auto kUniveralEndOfLineRegEx = QRegularExpression(QStringLiteral("\r\n|\r|\n")); } // anonymous namespace /** @@ -62,13 +65,6 @@ QList ParserM3u::parse(const QString& filename) { return paths; } - // Unfortunately QTextStream does not handle (=\r or asci value 13) line breaks. - // This is important on OS X where iTunes, e.g., exports M3U playlists using - // rather that . - // - // Using QFile::readAll() we obtain the complete content of the playlist as a ByteArray. - // We replace any '\r' with '\n' if applicaple - // This ensures that playlists from iTunes on OS X can be parsed QByteArray byteArray = file.readAll(); QString fileContents; if (isUtf8(byteArray.constData())) { @@ -80,7 +76,7 @@ QList ParserM3u::parse(const QString& filename) { } QFileInfo fileInfo(filename); - const QStringList fileLines = fileContents.split(QRegExp("\r\n|\r|\n")); + const QStringList fileLines = fileContents.split(kUniveralEndOfLineRegEx); for (const QString& line : fileLines) { auto trackFile = playlistEntryToFileInfo(line, fileInfo.canonicalPath()); if (trackFile.checkFileExists()) { From 0147ff4b9b8467cf0af899dd82272460adc47b78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 07:46:55 +0100 Subject: [PATCH 03/21] Added Test PlaylistTest.m3uEndOfLine --- src/library/parser.h | 2 +- src/library/parsercsv.cpp | 3 +- src/library/parsercsv.h | 11 +++----- src/library/parserm3u.cpp | 4 +-- src/library/parserm3u.h | 5 ++-- src/library/parserpls.cpp | 3 +- src/library/parserpls.h | 2 +- src/library/trackset/baseplaylistfeature.cpp | 2 +- src/library/trackset/crate/cratefeature.cpp | 6 ++-- src/test/playlisttest.cpp | 29 +++++++++++++++++++- src/util/dnd.cpp | 4 +-- 11 files changed, 48 insertions(+), 23 deletions(-) diff --git a/src/library/parser.h b/src/library/parser.h index ccad1acddb8..4995dc44fe4 100644 --- a/src/library/parser.h +++ b/src/library/parser.h @@ -41,7 +41,7 @@ class Parser : public QObject { Note for developers: This function should return an empty PtrList or 0 in order for the trackimporter to function**/ - virtual QList parse(const QString&) = 0; + virtual QList parse(const QString& sFilename, bool keepMissingFiles) = 0; protected: // Pointer to the parsed Filelocations diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index f34e5ea2d64..79b7013587b 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -27,7 +27,8 @@ ParserCsv::ParserCsv() : Parser() { ParserCsv::~ParserCsv() { } -QList ParserCsv::parse(const QString& sFilename) { +QList ParserCsv::parse(const QString& sFilename, bool keepMissingFiles) { + Q_UNUSED(keepMissingFiles); QFile file(sFilename); QString basepath = sFilename.section('/', 0, -2); diff --git a/src/library/parsercsv.h b/src/library/parsercsv.h index ecf1e1167ed..1514392800d 100644 --- a/src/library/parsercsv.h +++ b/src/library/parsercsv.h @@ -25,16 +25,13 @@ class ParserCsv : public Parser Q_OBJECT public: ParserCsv(); - virtual ~ParserCsv(); - /**Overwriting function parse in class Parser**/ - QList parse(const QString&); + ~ParserCsv() override; + QList parse(const QString&, bool keepMissingFiles) override; // Playlist Export static bool writeCSVFile(const QString &file, BaseSqlTableModel* pPlaylistTableModel, bool useRelativePath); // Readable Text export static bool writeReadableTextFile(const QString &file, BaseSqlTableModel* pPlaylistTableModel, bool writeTimestamp); private: - /**Reads a line from the file and returns filepath if a valid file**/ - QList > tokenize(const QByteArray& str, char delimiter); - - + // Reads a line from the file and returns filepath if a valid file + QList > tokenize(const QByteArray& str, char delimiter); }; diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index af5a9373b9e..0f71572bbb6 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -54,7 +54,7 @@ ParserM3u::~ParserM3u() } -QList ParserM3u::parse(const QString& filename) { +QList ParserM3u::parse(const QString& filename, bool keepMissingFiles) { QList paths; QFile file(filename); @@ -79,7 +79,7 @@ QList ParserM3u::parse(const QString& filename) { const QStringList fileLines = fileContents.split(kUniveralEndOfLineRegEx); for (const QString& line : fileLines) { auto trackFile = playlistEntryToFileInfo(line, fileInfo.canonicalPath()); - if (trackFile.checkFileExists()) { + if (keepMissingFiles || trackFile.checkFileExists()) { paths.append(trackFile.location()); } else { qInfo() << "File" << trackFile.location() << "from M3U playlist" diff --git a/src/library/parserm3u.h b/src/library/parserm3u.h index ea1d497a489..f37414cdda8 100644 --- a/src/library/parserm3u.h +++ b/src/library/parserm3u.h @@ -24,9 +24,8 @@ class ParserM3u : public Parser Q_OBJECT public: ParserM3u(); - ~ParserM3u(); - /**Overwriting function parse in class Parser**/ - QList parse(const QString&); + ~ParserM3u() override; + QList parse(const QString&, bool keepMissingFiles) override; //Playlist Export static bool writeM3UFile(const QString &file_str, const QList &items, bool useRelativePath, bool useUtf8); static bool writeM3UFile(const QString &file, const QList &items, bool useRelativePath); diff --git a/src/library/parserpls.cpp b/src/library/parserpls.cpp index 5691e96a630..90facad06bc 100644 --- a/src/library/parserpls.cpp +++ b/src/library/parserpls.cpp @@ -32,7 +32,8 @@ ParserPls::ParserPls() : Parser() { ParserPls::~ParserPls() { } -QList ParserPls::parse(const QString& sFilename) { +QList ParserPls::parse(const QString& sFilename, bool keepMissingFiles) { + Q_UNUSED(keepMissingFiles); //long numEntries =0; QFile file(sFilename); const auto basePath = sFilename.section('/', 0, -2); diff --git a/src/library/parserpls.h b/src/library/parserpls.h index a7ca089da3a..cfb4bc15daf 100644 --- a/src/library/parserpls.h +++ b/src/library/parserpls.h @@ -23,7 +23,7 @@ class ParserPls : public Parser { ParserPls(); virtual ~ParserPls(); /**Can be called to parse a pls file**/ - QList parse(const QString&); + QList parse(const QString& sFilename, bool keepMissingFiles); //Playlist Export static bool writePLSFile(const QString &file, const QList &items, bool useRelativePath); diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index c9e65d57861..1d7d4451a42 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -454,7 +454,7 @@ void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlist_file) { } if (playlist_parser) { - QStringList entries = playlist_parser->parse(playlist_file); + QStringList entries = playlist_parser->parse(playlist_file, false); // Iterate over the List that holds URLs of playlist entries m_pPlaylistTableModel->addTracks(QModelIndex(), entries); diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 35c5335ecee..b764e165859 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -610,11 +610,11 @@ void CrateFeature::slotImportPlaylistFile(const QString& playlist_file) { if (playlist_file.endsWith(".m3u", Qt::CaseInsensitive) || playlist_file.endsWith(".m3u8", Qt::CaseInsensitive)) { // .m3u8 is Utf8 representation of an m3u playlist - entries = ParserM3u().parse(playlist_file); + entries = ParserM3u().parse(playlist_file, false); } else if (playlist_file.endsWith(".pls", Qt::CaseInsensitive)) { - entries = ParserPls().parse(playlist_file); + entries = ParserPls().parse(playlist_file, false); } else if (playlist_file.endsWith(".csv", Qt::CaseInsensitive)) { - entries = ParserCsv().parse(playlist_file); + entries = ParserCsv().parse(playlist_file, false); } else { return; } diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index 3470403b49b..b1ace9243f6 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -1,14 +1,19 @@ #include +#include #include +#include #include #include #include "library/parser.h" +#include "library/parserm3u.h" class DummyParser : public Parser { public: - QList parse(const QString&) override { + QList parse(const QString& sFilename, bool keepMissingFiles) override { + Q_UNUSED(sFilename); + Q_UNUSED(keepMissingFiles); return QList(); } @@ -56,3 +61,25 @@ TEST_F(PlaylistTest, Relative) { EXPECT_EQ(QString("base/folder/../../bar.mp3"), parser.playlistEntryToFilePath("../../bar.mp3", "base/folder")); } + +TEST_F(PlaylistTest, m3uEndOfLine) { + QTemporaryFile m3uFile; + ASSERT_TRUE(m3uFile.open()); + m3uFile.write("crlf.mp3\r\n"); + m3uFile.write("cr.mp3\r"); + m3uFile.write("lf.mp3\n"); + // Check for Windows-1250 Euro Sign + m3uFile.write("EuroSign\x80.mp3\n"); + m3uFile.write("end.mp3"); + m3uFile.close(); + + QList entries = ParserM3u().parse(m3uFile.fileName(), true); + EXPECT_EQ(entries.size(), 5); + if (entries.size() == 5) { + EXPECT_TRUE(entries[0].endsWith(QStringLiteral("crlf.mp3"))); + EXPECT_TRUE(entries[1].endsWith(QStringLiteral("cr.mp3"))); + EXPECT_TRUE(entries[2].endsWith(QStringLiteral("lf.mp3"))); + EXPECT_TRUE(entries[3].endsWith(QStringLiteral("EuroSign\u20AC.mp3"))); + EXPECT_TRUE(entries[4].endsWith(QStringLiteral("end.mp3"))); + } +} diff --git a/src/util/dnd.cpp b/src/util/dnd.cpp index e201d0e677b..911cd78c5c8 100644 --- a/src/util/dnd.cpp +++ b/src/util/dnd.cpp @@ -117,13 +117,13 @@ QList DragAndDropHelper::supportedTracksFromUrls( if (acceptPlaylists && (file.endsWith(".m3u") || file.endsWith(".m3u8"))) { QScopedPointer playlist_parser(new ParserM3u()); - QList track_list = playlist_parser->parse(file); + QList track_list = playlist_parser->parse(file, false); foreach (const QString& playlistFile, track_list) { addFileToList(mixxx::FileInfo(playlistFile), &fileInfos); } } else if (acceptPlaylists && url.toString().endsWith(".pls")) { QScopedPointer playlist_parser(new ParserPls()); - QList track_list = playlist_parser->parse(file); + QList track_list = playlist_parser->parse(file, false); foreach (const QString& playlistFile, track_list) { addFileToList(mixxx::FileInfo(playlistFile), &fileInfos); } From 6c2ef37d5089564ec56b7f3e13bdc01d21e3bce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 08:09:36 +0100 Subject: [PATCH 04/21] skip m3u comments --- src/library/parserm3u.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index 0f71572bbb6..d54c60e2761 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -25,7 +25,9 @@ namespace { // according to http://en.wikipedia.org/wiki/M3U the default encoding of m3u is Windows-1252 // see also http://tools.ietf.org/html/draft-pantos-http-live-streaming-07 -const char* kStandardM3uTextEncoding = "Windows-1250"; +const char kStandardM3uTextEncoding[] = "Windows-1250"; +const char kM3uHeader[] = "#EXTM3U"; +const char kM3uCommentPrefix[] = "#"; // Note: The RegEx pattern is compiled, when first used the first time const auto kUniveralEndOfLineRegEx = QRegularExpression(QStringLiteral("\r\n|\r|\n")); } // anonymous namespace @@ -75,9 +77,17 @@ QList ParserM3u::parse(const QString& filename, bool keepMissingFiles) ->toUnicode(byteArray); } + if (!fileContents.startsWith(kM3uHeader)) { + qWarning() << "M3U playlist file" << filename << "does not start with" << kM3uHeader; + } + QFileInfo fileInfo(filename); const QStringList fileLines = fileContents.split(kUniveralEndOfLineRegEx); for (const QString& line : fileLines) { + if (line.startsWith(kM3uCommentPrefix)) { + // Skip lines with comments + continue; + } auto trackFile = playlistEntryToFileInfo(line, fileInfo.canonicalPath()); if (keepMissingFiles || trackFile.checkFileExists()) { paths.append(trackFile.location()); From 4340fc45b1437a98c687b9180ebae218f5dbdd1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 17:46:00 +0100 Subject: [PATCH 05/21] Rename sFilename to playlistFile --- src/library/parser.h | 2 +- src/library/parsercsv.cpp | 6 +++--- src/library/parserm3u.cpp | 12 ++++++------ src/library/parserm3u.h | 4 ++-- src/library/parserpls.cpp | 7 +++---- src/library/parserpls.h | 10 +++++----- 6 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/library/parser.h b/src/library/parser.h index 4995dc44fe4..8ec2bf6df70 100644 --- a/src/library/parser.h +++ b/src/library/parser.h @@ -41,7 +41,7 @@ class Parser : public QObject { Note for developers: This function should return an empty PtrList or 0 in order for the trackimporter to function**/ - virtual QList parse(const QString& sFilename, bool keepMissingFiles) = 0; + virtual QList parse(const QString& playlistFile, bool keepMissingFiles) = 0; protected: // Pointer to the parsed Filelocations diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index 79b7013587b..63f082f1d6a 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -27,10 +27,10 @@ ParserCsv::ParserCsv() : Parser() { ParserCsv::~ParserCsv() { } -QList ParserCsv::parse(const QString& sFilename, bool keepMissingFiles) { +QList ParserCsv::parse(const QString& playlistFile, bool keepMissingFiles) { Q_UNUSED(keepMissingFiles); - QFile file(sFilename); - QString basepath = sFilename.section('/', 0, -2); + QFile file(playlistFile); + QString basepath = playlistFile.section('/', 0, -2); clearLocations(); //qDebug() << "ParserCsv: Starting to parse."; diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index d54c60e2761..fc40f5bec21 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -56,14 +56,14 @@ ParserM3u::~ParserM3u() } -QList ParserM3u::parse(const QString& filename, bool keepMissingFiles) { +QList ParserM3u::parse(const QString& playlistFile, bool keepMissingFiles) { QList paths; - QFile file(filename); + QFile file(playlistFile); if (!file.open(QIODevice::ReadOnly)) { qWarning() << "Failed to open playlist file" - << filename; + << playlistFile; return paths; } @@ -78,10 +78,10 @@ QList ParserM3u::parse(const QString& filename, bool keepMissingFiles) } if (!fileContents.startsWith(kM3uHeader)) { - qWarning() << "M3U playlist file" << filename << "does not start with" << kM3uHeader; + qWarning() << "M3U playlist file" << playlistFile << "does not start with" << kM3uHeader; } - QFileInfo fileInfo(filename); + QFileInfo fileInfo(playlistFile); const QStringList fileLines = fileContents.split(kUniveralEndOfLineRegEx); for (const QString& line : fileLines) { if (line.startsWith(kM3uCommentPrefix)) { @@ -93,7 +93,7 @@ QList ParserM3u::parse(const QString& filename, bool keepMissingFiles) paths.append(trackFile.location()); } else { qInfo() << "File" << trackFile.location() << "from M3U playlist" - << filename << "does not exist."; + << playlistFile << "does not exist."; } } diff --git a/src/library/parserm3u.h b/src/library/parserm3u.h index f37414cdda8..203b62ce048 100644 --- a/src/library/parserm3u.h +++ b/src/library/parserm3u.h @@ -25,8 +25,8 @@ class ParserM3u : public Parser public: ParserM3u(); ~ParserM3u() override; - QList parse(const QString&, bool keepMissingFiles) override; - //Playlist Export + QList parse(const QString& playlistFile, bool keepMissingFiles) override; + /// Playlist Export static bool writeM3UFile(const QString &file_str, const QList &items, bool useRelativePath, bool useUtf8); static bool writeM3UFile(const QString &file, const QList &items, bool useRelativePath); static bool writeM3U8File(const QString &file_str, const QList &items, bool useRelativePath); diff --git a/src/library/parserpls.cpp b/src/library/parserpls.cpp index 90facad06bc..8a9f884d27e 100644 --- a/src/library/parserpls.cpp +++ b/src/library/parserpls.cpp @@ -32,11 +32,10 @@ ParserPls::ParserPls() : Parser() { ParserPls::~ParserPls() { } -QList ParserPls::parse(const QString& sFilename, bool keepMissingFiles) { +QList ParserPls::parse(const QString& playlistFile, bool keepMissingFiles) { Q_UNUSED(keepMissingFiles); - //long numEntries =0; - QFile file(sFilename); - const auto basePath = sFilename.section('/', 0, -2); + QFile file(playlistFile); + const auto basePath = playlistFile.section('/', 0, -2); clearLocations(); diff --git a/src/library/parserpls.h b/src/library/parserpls.h index cfb4bc15daf..e996b9fa2ec 100644 --- a/src/library/parserpls.h +++ b/src/library/parserpls.h @@ -22,15 +22,15 @@ class ParserPls : public Parser { public: ParserPls(); virtual ~ParserPls(); - /**Can be called to parse a pls file**/ - QList parse(const QString& sFilename, bool keepMissingFiles); - //Playlist Export + /// Can be called to parse a pls file + QList parse(const QString& playlistFile, bool keepMissingFiles); + /// Playlist Export static bool writePLSFile(const QString &file, const QList &items, bool useRelativePath); private: - /**Returns the Number of entries in the pls file**/ + /// Returns the Number of entries in the pls file long getNumEntries(QTextStream*); - /**Reads a line from the file and returns filepath**/ + /// Reads a line from the file and returns filepath QString getFilePath(QTextStream*, const QString& basePath); }; From 66635154f6924a7b90834969ae8f18164a8f509b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 17:57:28 +0100 Subject: [PATCH 06/21] Remove unused functions from Parser --- src/library/parser.cpp | 9 +-------- src/library/parser.h | 12 +----------- src/library/parsercsv.cpp | 19 +++++-------------- src/library/parserm3u.cpp | 4 ++-- src/library/parserpls.cpp | 17 ++++------------- src/test/playlisttest.cpp | 2 +- 6 files changed, 14 insertions(+), 49 deletions(-) diff --git a/src/library/parser.cpp b/src/library/parser.cpp index 4e7034038bf..f5f940313b2 100644 --- a/src/library/parser.cpp +++ b/src/library/parser.cpp @@ -25,14 +25,6 @@ Parser::Parser() { Parser::~Parser() { } -void Parser::clearLocations() { - m_sLocations.clear(); -} - -long Parser::countParsed() { - return (long)m_sLocations.count(); -} - // The following public domain code is taken from // http://stackoverflow.com/questions/1031645/how-to-detect-utf-8-in-plain-c // Thank you Christoph! @@ -115,6 +107,7 @@ bool Parser::isUtf8(const char* string) { return true; } +// static mixxx::FileInfo Parser::playlistEntryToFileInfo( const QString& playlistEntry, const QString& basePath) { diff --git a/src/library/parser.h b/src/library/parser.h index 8ec2bf6df70..8cb3260638f 100644 --- a/src/library/parser.h +++ b/src/library/parser.h @@ -37,23 +37,13 @@ class Parser : public QObject { Parser(); ~Parser() override; - /**Can be called to parse a pls file - Note for developers: - This function should return an empty PtrList - or 0 in order for the trackimporter to function**/ virtual QList parse(const QString& playlistFile, bool keepMissingFiles) = 0; protected: - // Pointer to the parsed Filelocations - QList m_sLocations; - // Returns the number of parsed locations - long countParsed(); - // Clears m_psLocations - void clearLocations(); // check for Utf8 encoding static bool isUtf8(const char* string); // Resolve an absolute or relative file path - mixxx::FileInfo playlistEntryToFileInfo( + static mixxx::FileInfo playlistEntryToFileInfo( const QString& playlistEntry, const QString& basePath = QString()); }; diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index 63f082f1d6a..4180c221360 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -32,7 +32,7 @@ QList ParserCsv::parse(const QString& playlistFile, bool keepMissingFil QFile file(playlistFile); QString basepath = playlistFile.section('/', 0, -2); - clearLocations(); + QList locations; //qDebug() << "ParserCsv: Starting to parse."; if (file.open(QIODevice::ReadOnly)) { QByteArray ba = file.readAll(); @@ -57,22 +57,13 @@ QList ParserCsv::parse(const QString& playlistFile, bool keepMissingFil qDebug() << "is relative" << basepath << fi.filePath(); fi.setFile(basepath,fi.filePath()); } - m_sLocations.append(fi.filePath()); + locations.append(fi.filePath()); } } } - file.close(); - - if (m_sLocations.count() != 0) { - return m_sLocations; - } else { - return QList(); // NULL pointer returned when no locations were found - } } - - file.close(); - return QList(); //if we get here something went wrong + return locations; } // Code was posted at http://www.qtcentre.org/threads/35511-Parsing-CSV-data @@ -98,14 +89,14 @@ QList > ParserCsv::tokenize(const QByteArray& str, char delimiter quotes = false; } } else if (!quotes && c == delimiter) { - if (isUtf8(field.constData())) { + if (Parser::isUtf8(field.constData())) { tokens[row].append(QString::fromUtf8(field)); } else { tokens[row].append(QString::fromLatin1(field)); } field.clear(); } else if (!quotes && (c == '\r' || c == '\n')) { - if (isUtf8(field.constData())) { + if (Parser::isUtf8(field.constData())) { tokens[row].append(QString::fromUtf8(field)); } else { tokens[row].append(QString::fromLatin1(field)); diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index fc40f5bec21..cafd308e26a 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -69,7 +69,7 @@ QList ParserM3u::parse(const QString& playlistFile, bool keepMissingFil QByteArray byteArray = file.readAll(); QString fileContents; - if (isUtf8(byteArray.constData())) { + if (Parser::isUtf8(byteArray.constData())) { fileContents = QString::fromUtf8(byteArray); } else { // FIXME: replace deprecated QTextCodec with direct usage of libicu @@ -88,7 +88,7 @@ QList ParserM3u::parse(const QString& playlistFile, bool keepMissingFil // Skip lines with comments continue; } - auto trackFile = playlistEntryToFileInfo(line, fileInfo.canonicalPath()); + auto trackFile = Parser::playlistEntryToFileInfo(line, fileInfo.canonicalPath()); if (keepMissingFiles || trackFile.checkFileExists()) { paths.append(trackFile.location()); } else { diff --git a/src/library/parserpls.cpp b/src/library/parserpls.cpp index 8a9f884d27e..2789fedcec6 100644 --- a/src/library/parserpls.cpp +++ b/src/library/parserpls.cpp @@ -37,8 +37,7 @@ QList ParserPls::parse(const QString& playlistFile, bool keepMissingFil QFile file(playlistFile); const auto basePath = playlistFile.section('/', 0, -2); - clearLocations(); - + QList locations; if (file.open(QIODevice::ReadOnly)) { /* Unfortunately, QT 4.7 does not handle (=\r or asci value 13) line breaks. * This is important on OS X where iTunes, e.g., exports M3U playlists using @@ -62,22 +61,14 @@ QList ParserPls::parse(const QString& playlistFile, bool keepMissingFil if(psLine.isNull()) { break; } else { - m_sLocations.append(psLine); + locations.append(psLine); } } file.close(); - - if (m_sLocations.count() != 0) { - return m_sLocations; - } else { - return QList(); // NULL pointer returned when no locations were found - } } - - file.close(); - return QList(); //if we get here something went wrong :D + return locations; } long ParserPls::getNumEntries(QTextStream *stream) { @@ -113,7 +104,7 @@ QString ParserPls::getFilePath(QTextStream *stream, const QString& basePath) { ++iPos; QString filename = textline.right(textline.length() - iPos); - auto trackFile = playlistEntryToFileInfo(filename, basePath); + auto trackFile = Parser::playlistEntryToFileInfo(filename, basePath); if (trackFile.checkFileExists()) { return trackFile.location(); } diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index b1ace9243f6..d0ffd5e118c 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -20,7 +20,7 @@ class DummyParser : public Parser { QString playlistEntryToFilePath( const QString& playlistEntry, const QString& basePath = QString()) { - const auto fileInfo = playlistEntryToFileInfo(playlistEntry, basePath); + const auto fileInfo = Parser::playlistEntryToFileInfo(playlistEntry, basePath); // Return the plain, literal file path, because the location // is undefined if relative paths. return fileInfo.asQFileInfo().filePath(); From 1f71a5d6b58bfb16fdb9b020cf5d6d1618d0e2bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 19:34:51 +0100 Subject: [PATCH 07/21] Pipe all parse calls through static Parser::parse() --- src/library/parser.cpp | 37 ++++++++------ src/library/parser.h | 14 ++--- src/library/parsercsv.cpp | 34 ++++-------- src/library/parsercsv.h | 19 ++++--- src/library/parserm3u.cpp | 16 +++--- src/library/parserm3u.h | 11 ++-- src/library/parserpls.cpp | 54 +++----------------- src/library/parserpls.h | 18 +++---- src/library/trackset/baseplaylistfeature.cpp | 25 ++------- src/library/trackset/crate/cratefeature.cpp | 14 ++--- src/test/playlisttest.cpp | 6 --- src/util/dnd.cpp | 13 ++--- 12 files changed, 76 insertions(+), 185 deletions(-) diff --git a/src/library/parser.cpp b/src/library/parser.cpp index f5f940313b2..d6aa14de28d 100644 --- a/src/library/parser.cpp +++ b/src/library/parser.cpp @@ -1,28 +1,33 @@ -// -// C++ Implementation: parser -// -// Description: superclass for external formats parsers -// -// -// Author: Ingo Kossyk , (C) 2004 -// Author: Tobias Rafreider trafreider@mixxx.org, (C) 2011 -// -// Copyright: See COPYING file that comes with this distribution -// -// +#include "library/parser.h" -#include #include #include #include #include +#include -#include "library/parser.h" +#include "library/parsercsv.h" +#include "library/parserm3u.h" +#include "library/parserpls.h" -Parser::Parser() { +// static +bool Parser::isPlaylistFilenameSupported(const QString& playlistFile) { + return ParserM3u::isPlaylistFilenameSupported(playlistFile) || + ParserPls::isPlaylistFilenameSupported(playlistFile) || + ParserCsv::isPlaylistFilenameSupported(playlistFile); } -Parser::~Parser() { +// static +QList Parser::parse(const QString& playlistFile, bool keepMissingFiles) { + QList locations; + if (ParserM3u::isPlaylistFilenameSupported(playlistFile)) { + locations = ParserM3u::parse(playlistFile, keepMissingFiles); + } else if (ParserPls::isPlaylistFilenameSupported(playlistFile)) { + locations = ParserM3u::parse(playlistFile, keepMissingFiles); + } else if (ParserCsv::isPlaylistFilenameSupported(playlistFile)) { + locations = ParserM3u::parse(playlistFile, keepMissingFiles); + } + return locations; } // The following public domain code is taken from diff --git a/src/library/parser.h b/src/library/parser.h index 8cb3260638f..e0ae04ae8e0 100644 --- a/src/library/parser.h +++ b/src/library/parser.h @@ -26,18 +26,10 @@ it afterwards for proper functioning #include "util/fileinfo.h" -class Parser : public QObject { +class Parser { public: - static bool isPlaylistFilenameSupported(const QString& fileName) { - return fileName.endsWith(".m3u", Qt::CaseInsensitive) || - fileName.endsWith(".m3u8", Qt::CaseInsensitive) || - fileName.endsWith(".pls", Qt::CaseInsensitive) || - fileName.endsWith(".csv", Qt::CaseInsensitive); - } - - Parser(); - ~Parser() override; - virtual QList parse(const QString& playlistFile, bool keepMissingFiles) = 0; + static bool isPlaylistFilenameSupported(const QString& playlistFile); + static QList parse(const QString& playlistFile, bool keepMissingFiles); protected: // check for Utf8 encoding diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index 4180c221360..5900fd29186 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -1,17 +1,3 @@ -// -// C++ Implementation: parsercsv -// -// Description: module to parse Comma-Separated Values (CSV) formatted playlists (rfc4180) -// -// -// Author: Ingo Kossyk , (C) 2004 -// Author: Tobias Rafreider trafreider@mixxx.org, (C) 2011 -// Author: Daniel Schürmann daschuer@gmx.de, (C) 2011 -// -// Copyright: See COPYING file that comes with this distribution -// -// - #include "library/parsercsv.h" #include @@ -19,14 +5,14 @@ #include #include -#include "moc_parsercsv.cpp" - -ParserCsv::ParserCsv() : Parser() { -} +#include "library/parser.h" -ParserCsv::~ParserCsv() { +// static +bool ParserCsv::isPlaylistFilenameSupported(const QString& playlistFile) { + return playlistFile.endsWith(".csv", Qt::CaseInsensitive); } +// static QList ParserCsv::parse(const QString& playlistFile, bool keepMissingFiles) { Q_UNUSED(keepMissingFiles); QFile file(playlistFile); @@ -43,7 +29,7 @@ QList ParserCsv::parse(const QString& playlistFile, bool keepMissingFil int loc_coll = 0x7fffffff; if (tokens.size()) { for (int i = 0; i < tokens[0].size(); ++i) { - if (tokens[0][i] == tr("Location")) { + if (tokens[0][i] == QObject::tr("Location")) { loc_coll = i; break; } @@ -122,8 +108,8 @@ bool ParserCsv::writeCSVFile(const QString &file_str, BaseSqlTableModel* pPlayli QFile file(file_str); if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { QMessageBox::warning(nullptr, - tr("Playlist Export Failed"), - tr("Could not create file") + " " + file_str); + QObject::tr("Playlist Export Failed"), + QObject::tr("Could not create file") + " " + file_str); return false; } //Base folder of file @@ -213,8 +199,8 @@ bool ParserCsv::writeReadableTextFile(const QString &file_str, BaseSqlTableModel QFile file(file_str); if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { QMessageBox::warning(nullptr, - tr("Readable text Export Failed"), - tr("Could not create file") + " " + file_str); + QObject::tr("Readable text Export Failed"), + QObject::tr("Could not create file") + " " + file_str); return false; } diff --git a/src/library/parsercsv.h b/src/library/parsercsv.h index 1514392800d..19047695440 100644 --- a/src/library/parsercsv.h +++ b/src/library/parsercsv.h @@ -20,18 +20,17 @@ #include "library/parser.h" #include "library/basesqltablemodel.h" -class ParserCsv : public Parser -{ - Q_OBJECT -public: - ParserCsv(); - ~ParserCsv() override; - QList parse(const QString&, bool keepMissingFiles) override; +class ParserCsv : Parser { + public: + // static + static bool isPlaylistFilenameSupported(const QString& playlistFile); + static QList parse(const QString&, bool keepMissingFiles); // Playlist Export static bool writeCSVFile(const QString &file, BaseSqlTableModel* pPlaylistTableModel, bool useRelativePath); // Readable Text export static bool writeReadableTextFile(const QString &file, BaseSqlTableModel* pPlaylistTableModel, bool writeTimestamp); -private: - // Reads a line from the file and returns filepath if a valid file - QList > tokenize(const QByteArray& str, char delimiter); + + private: + // Reads a line from the file and returns filepath if a valid file + static QList > tokenize(const QByteArray& str, char delimiter); }; diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index cafd308e26a..f13731bebbf 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -20,7 +20,6 @@ #include #include -#include "moc_parserm3u.cpp" namespace { // according to http://en.wikipedia.org/wiki/M3U the default encoding of m3u is Windows-1252 @@ -47,13 +46,10 @@ const auto kUniveralEndOfLineRegEx = QRegularExpression(QStringLiteral("\r\n|\r| or on a mounted harddrive. **/ -ParserM3u::ParserM3u() : Parser() -{ -} - -ParserM3u::~ParserM3u() -{ - +// static +bool ParserM3u::isPlaylistFilenameSupported(const QString& fileName) { + return fileName.endsWith(".m3u", Qt::CaseInsensitive) || + fileName.endsWith(".m3u8", Qt::CaseInsensitive); } QList ParserM3u::parse(const QString& playlistFile, bool keepMissingFiles) { @@ -138,8 +134,8 @@ bool ParserM3u::writeM3UFile(const QString &file_str, const QList &item QFile file(file_str); if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { QMessageBox::warning(nullptr, - tr("Playlist Export Failed"), - tr("Could not create file") + " " + file_str); + QObject::tr("Playlist Export Failed"), + QObject::tr("Could not create file") + " " + file_str); return false; } file.write(outputByteArray); diff --git a/src/library/parserm3u.h b/src/library/parserm3u.h index 203b62ce048..3b3360d92fd 100644 --- a/src/library/parserm3u.h +++ b/src/library/parserm3u.h @@ -19,13 +19,10 @@ #include "library/parser.h" -class ParserM3u : public Parser -{ - Q_OBJECT -public: - ParserM3u(); - ~ParserM3u() override; - QList parse(const QString& playlistFile, bool keepMissingFiles) override; +class ParserM3u : Parser { + public: + static bool isPlaylistFilenameSupported(const QString& fileName); + static QList parse(const QString& playlistFile, bool keepMissingFiles); /// Playlist Export static bool writeM3UFile(const QString &file_str, const QList &items, bool useRelativePath, bool useUtf8); static bool writeM3UFile(const QString &file, const QList &items, bool useRelativePath); diff --git a/src/library/parserpls.cpp b/src/library/parserpls.cpp index 2789fedcec6..ace73040037 100644 --- a/src/library/parserpls.cpp +++ b/src/library/parserpls.cpp @@ -1,15 +1,3 @@ -// -// C++ Implementation: parserpls -// -// Description: module to parse pls formatted playlists -// -// -// Author: Ingo Kossyk , (C) 2004 -// Author: Tobias Rafreider trafreider@mixxx.org, (C) 2011 -// -// Copyright: See COPYING file that comes with this distribution -// -// #include "library/parserpls.h" #include @@ -18,20 +6,14 @@ #include #include -#include "moc_parserpls.cpp" +#include "library/parser.h" -/** - ToDo: - - parse ALL information from the pls file if available , - not only the filepath; - **/ - -ParserPls::ParserPls() : Parser() { -} - -ParserPls::~ParserPls() { +// static +bool ParserPls::isPlaylistFilenameSupported(const QString& playlistFile) { + return playlistFile.endsWith(".pls", Qt::CaseInsensitive); } +// static QList ParserPls::parse(const QString& playlistFile, bool keepMissingFiles) { Q_UNUSED(keepMissingFiles); QFile file(playlistFile); @@ -71,27 +53,7 @@ QList ParserPls::parse(const QString& playlistFile, bool keepMissingFil return locations; } -long ParserPls::getNumEntries(QTextStream *stream) { - QString textline; - textline = stream->readLine(); - - if (textline.contains("[playlist]")) { - while (!textline.contains("NumberOfEntries")) { - textline = stream->readLine(); - } - - QString temp = textline.section("=",-1,-1); - - return temp.toLong(); - - } else{ - qDebug() << "ParserPls: pls file is not a playlist! \n"; - return 0; - } - -} - - +// static QString ParserPls::getFilePath(QTextStream *stream, const QString& basePath) { QString textline = stream->readLine(); while (!textline.isEmpty()) { @@ -123,8 +85,8 @@ bool ParserPls::writePLSFile(const QString &file_str, const QList &item QFile file(file_str); if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { QMessageBox::warning(nullptr, - tr("Playlist Export Failed"), - tr("Could not create file") + " " + file_str); + QObject::tr("Playlist Export Failed"), + QObject::tr("Could not create file") + " " + file_str); return false; } //Base folder of file diff --git a/src/library/parserpls.h b/src/library/parserpls.h index e996b9fa2ec..eea00b6e857 100644 --- a/src/library/parserpls.h +++ b/src/library/parserpls.h @@ -11,26 +11,20 @@ // #pragma once -#include "library/parser.h" - -#include #include #include +#include -class ParserPls : public Parser { - Q_OBJECT +#include "library/parser.h" + +class ParserPls : Parser { public: - ParserPls(); - virtual ~ParserPls(); - /// Can be called to parse a pls file + static bool isPlaylistFilenameSupported(const QString& fileName); QList parse(const QString& playlistFile, bool keepMissingFiles); /// Playlist Export static bool writePLSFile(const QString &file, const QList &items, bool useRelativePath); private: - /// Returns the Number of entries in the pls file - long getNumEntries(QTextStream*); /// Reads a line from the file and returns filepath - QString getFilePath(QTextStream*, const QString& basePath); - + static QString getFilePath(QTextStream*, const QString& basePath); }; diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 1d7d4451a42..9f640b589c6 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -440,28 +440,9 @@ void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlist_file) { // folder. We don't need access to this file on a regular basis so we do not // register a security bookmark. - Parser* playlist_parser = nullptr; - - if (playlist_file.endsWith(".m3u", Qt::CaseInsensitive) || - playlist_file.endsWith(".m3u8", Qt::CaseInsensitive)) { - playlist_parser = new ParserM3u(); - } else if (playlist_file.endsWith(".pls", Qt::CaseInsensitive)) { - playlist_parser = new ParserPls(); - } else if (playlist_file.endsWith(".csv", Qt::CaseInsensitive)) { - playlist_parser = new ParserCsv(); - } else { - return; - } - - if (playlist_parser) { - QStringList entries = playlist_parser->parse(playlist_file, false); - - // Iterate over the List that holds URLs of playlist entries - m_pPlaylistTableModel->addTracks(QModelIndex(), entries); - - // delete the parser object - delete playlist_parser; - } + QList locations = Parser::parse(playlist_file, false); + // Iterate over the List that holds locations of playlist entries + m_pPlaylistTableModel->addTracks(QModelIndex(), locations); } void BasePlaylistFeature::slotCreateImportPlaylist() { diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index b764e165859..29cbe5db732 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -606,19 +606,11 @@ void CrateFeature::slotImportPlaylistFile(const QString& playlist_file) { // register a security bookmark. // TODO(XXX): Parsing a list of track locations from a playlist file // is a general task and should be implemented separately. - QList entries; - if (playlist_file.endsWith(".m3u", Qt::CaseInsensitive) || - playlist_file.endsWith(".m3u8", Qt::CaseInsensitive)) { - // .m3u8 is Utf8 representation of an m3u playlist - entries = ParserM3u().parse(playlist_file, false); - } else if (playlist_file.endsWith(".pls", Qt::CaseInsensitive)) { - entries = ParserPls().parse(playlist_file, false); - } else if (playlist_file.endsWith(".csv", Qt::CaseInsensitive)) { - entries = ParserCsv().parse(playlist_file, false); - } else { + QList locations = Parser().parse(playlist_file, false); + if (locations.empty()) { return; } - m_crateTableModel.addTracks(QModelIndex(), entries); + m_crateTableModel.addTracks(QModelIndex(), locations); } void CrateFeature::slotCreateImportCrate() { diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index d0ffd5e118c..e73e6dd9c1b 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -11,12 +11,6 @@ class DummyParser : public Parser { public: - QList parse(const QString& sFilename, bool keepMissingFiles) override { - Q_UNUSED(sFilename); - Q_UNUSED(keepMissingFiles); - return QList(); - } - QString playlistEntryToFilePath( const QString& playlistEntry, const QString& basePath = QString()) { diff --git a/src/util/dnd.cpp b/src/util/dnd.cpp index 911cd78c5c8..da83dd516b4 100644 --- a/src/util/dnd.cpp +++ b/src/util/dnd.cpp @@ -115,16 +115,9 @@ QList DragAndDropHelper::supportedTracksFromUrls( continue; } - if (acceptPlaylists && (file.endsWith(".m3u") || file.endsWith(".m3u8"))) { - QScopedPointer playlist_parser(new ParserM3u()); - QList track_list = playlist_parser->parse(file, false); - foreach (const QString& playlistFile, track_list) { - addFileToList(mixxx::FileInfo(playlistFile), &fileInfos); - } - } else if (acceptPlaylists && url.toString().endsWith(".pls")) { - QScopedPointer playlist_parser(new ParserPls()); - QList track_list = playlist_parser->parse(file, false); - foreach (const QString& playlistFile, track_list) { + if (acceptPlaylists && Parser::isPlaylistFilenameSupported(file)) { + const QList track_list = Parser::parse(file, false); + for (auto& playlistFile : track_list) { addFileToList(mixxx::FileInfo(playlistFile), &fileInfos); } } else { From b5864856089cacbf241434a6221f24f7f3ed2de0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 21:42:32 +0100 Subject: [PATCH 08/21] Move file existence check to the common Parser::parse() function --- src/library/parser.cpp | 24 +++++++-- src/library/parser.h | 2 +- src/library/parsercsv.cpp | 20 +++----- src/library/parsercsv.h | 2 +- src/library/parserm3u.cpp | 12 +---- src/library/parserm3u.h | 2 +- src/library/parserpls.cpp | 54 +++++++++----------- src/library/parserpls.h | 6 +-- src/library/trackset/baseplaylistfeature.cpp | 2 +- src/library/trackset/crate/cratefeature.cpp | 2 +- src/test/playlisttest.cpp | 2 +- src/util/dnd.cpp | 2 +- 12 files changed, 60 insertions(+), 70 deletions(-) diff --git a/src/library/parser.cpp b/src/library/parser.cpp index d6aa14de28d..c0fc383ed4d 100644 --- a/src/library/parser.cpp +++ b/src/library/parser.cpp @@ -18,16 +18,30 @@ bool Parser::isPlaylistFilenameSupported(const QString& playlistFile) { } // static -QList Parser::parse(const QString& playlistFile, bool keepMissingFiles) { +QList Parser::parse(const QString& playlistFile) { QList locations; if (ParserM3u::isPlaylistFilenameSupported(playlistFile)) { - locations = ParserM3u::parse(playlistFile, keepMissingFiles); + locations = ParserM3u::parse(playlistFile); } else if (ParserPls::isPlaylistFilenameSupported(playlistFile)) { - locations = ParserM3u::parse(playlistFile, keepMissingFiles); + locations = ParserPls::parse(playlistFile); } else if (ParserCsv::isPlaylistFilenameSupported(playlistFile)) { - locations = ParserM3u::parse(playlistFile, keepMissingFiles); + locations = ParserCsv::parse(playlistFile); } - return locations; + + QFileInfo fileInfo(playlistFile); + + QList existingLocations; + for (auto& location : locations) { + mixxx::FileInfo trackFile = Parser::playlistEntryToFileInfo( + location, fileInfo.canonicalPath()); + if (trackFile.checkFileExists()) { + existingLocations.append(trackFile.location()); + } else { + qInfo() << "File" << trackFile.location() << "from playlist" + << playlistFile << "does not exist."; + } + } + return existingLocations; } // The following public domain code is taken from diff --git a/src/library/parser.h b/src/library/parser.h index e0ae04ae8e0..98630078eb9 100644 --- a/src/library/parser.h +++ b/src/library/parser.h @@ -29,7 +29,7 @@ it afterwards for proper functioning class Parser { public: static bool isPlaylistFilenameSupported(const QString& playlistFile); - static QList parse(const QString& playlistFile, bool keepMissingFiles); + static QList parse(const QString& playlistFile); protected: // check for Utf8 encoding diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index 5900fd29186..7ca0466916a 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -13,8 +13,7 @@ bool ParserCsv::isPlaylistFilenameSupported(const QString& playlistFile) { } // static -QList ParserCsv::parse(const QString& playlistFile, bool keepMissingFiles) { - Q_UNUSED(keepMissingFiles); +QList ParserCsv::parse(const QString& playlistFile) { QFile file(playlistFile); QString basepath = playlistFile.section('/', 0, -2); @@ -26,24 +25,19 @@ QList ParserCsv::parse(const QString& playlistFile, bool keepMissingFil QList > tokens = tokenize(ba, ','); // detect Location column - int loc_coll = 0x7fffffff; + int loc_col = 0x7fffffff; if (tokens.size()) { for (int i = 0; i < tokens[0].size(); ++i) { if (tokens[0][i] == QObject::tr("Location")) { - loc_coll = i; + loc_col = i; break; } } for (int i = 1; i < tokens.size(); ++i) { - if (loc_coll < tokens[i].size()) { - // Todo: check if path is relative - QFileInfo fi(tokens[i][loc_coll]); - if (fi.isRelative()) { - // add base path - qDebug() << "is relative" << basepath << fi.filePath(); - fi.setFile(basepath,fi.filePath()); - } - locations.append(fi.filePath()); + qDebug() << tokens; + if (loc_col < tokens[i].size()) { + qDebug() << tokens[i][loc_col]; + locations.append(tokens[i][loc_col]); } } } diff --git a/src/library/parsercsv.h b/src/library/parsercsv.h index 19047695440..f84818dfb21 100644 --- a/src/library/parsercsv.h +++ b/src/library/parsercsv.h @@ -24,7 +24,7 @@ class ParserCsv : Parser { public: // static static bool isPlaylistFilenameSupported(const QString& playlistFile); - static QList parse(const QString&, bool keepMissingFiles); + static QList parse(const QString&); // Playlist Export static bool writeCSVFile(const QString &file, BaseSqlTableModel* pPlaylistTableModel, bool useRelativePath); // Readable Text export diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index f13731bebbf..0002492f345 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -52,7 +52,7 @@ bool ParserM3u::isPlaylistFilenameSupported(const QString& fileName) { fileName.endsWith(".m3u8", Qt::CaseInsensitive); } -QList ParserM3u::parse(const QString& playlistFile, bool keepMissingFiles) { +QList ParserM3u::parse(const QString& playlistFile) { QList paths; QFile file(playlistFile); @@ -77,22 +77,14 @@ QList ParserM3u::parse(const QString& playlistFile, bool keepMissingFil qWarning() << "M3U playlist file" << playlistFile << "does not start with" << kM3uHeader; } - QFileInfo fileInfo(playlistFile); const QStringList fileLines = fileContents.split(kUniveralEndOfLineRegEx); for (const QString& line : fileLines) { if (line.startsWith(kM3uCommentPrefix)) { // Skip lines with comments continue; } - auto trackFile = Parser::playlistEntryToFileInfo(line, fileInfo.canonicalPath()); - if (keepMissingFiles || trackFile.checkFileExists()) { - paths.append(trackFile.location()); - } else { - qInfo() << "File" << trackFile.location() << "from M3U playlist" - << playlistFile << "does not exist."; - } + paths.append(line); } - return paths; } diff --git a/src/library/parserm3u.h b/src/library/parserm3u.h index 3b3360d92fd..594d3de0851 100644 --- a/src/library/parserm3u.h +++ b/src/library/parserm3u.h @@ -22,7 +22,7 @@ class ParserM3u : Parser { public: static bool isPlaylistFilenameSupported(const QString& fileName); - static QList parse(const QString& playlistFile, bool keepMissingFiles); + static QList parse(const QString& playlistFile); /// Playlist Export static bool writeM3UFile(const QString &file_str, const QList &items, bool useRelativePath, bool useUtf8); static bool writeM3UFile(const QString &file, const QList &items, bool useRelativePath); diff --git a/src/library/parserpls.cpp b/src/library/parserpls.cpp index ace73040037..c8191235db6 100644 --- a/src/library/parserpls.cpp +++ b/src/library/parserpls.cpp @@ -8,14 +8,35 @@ #include "library/parser.h" +namespace { + +QString getFilePath(QTextStream* stream) { + QString textline = stream->readLine(); + while (!textline.isEmpty()) { + if (textline.isNull()) { + break; + } + + if (textline.contains("File")) { + int iPos = textline.indexOf("=", 0); + ++iPos; + return textline.right(textline.length() - iPos); + } + textline = stream->readLine(); + } + // Signal we reached the end + return QString(); +} + +} // namespace + // static bool ParserPls::isPlaylistFilenameSupported(const QString& playlistFile) { return playlistFile.endsWith(".pls", Qt::CaseInsensitive); } // static -QList ParserPls::parse(const QString& playlistFile, bool keepMissingFiles) { - Q_UNUSED(keepMissingFiles); +QList ParserPls::parse(const QString& playlistFile) { QFile file(playlistFile); const auto basePath = playlistFile.section('/', 0, -2); @@ -39,7 +60,7 @@ QList ParserPls::parse(const QString& playlistFile, bool keepMissingFil QTextStream textstream(ba.constData()); while(!textstream.atEnd()) { - QString psLine = getFilePath(&textstream, basePath); + QString psLine = getFilePath(&textstream); if(psLine.isNull()) { break; } else { @@ -53,33 +74,6 @@ QList ParserPls::parse(const QString& playlistFile, bool keepMissingFil return locations; } -// static -QString ParserPls::getFilePath(QTextStream *stream, const QString& basePath) { - QString textline = stream->readLine(); - while (!textline.isEmpty()) { - if (textline.isNull()) { - break; - } - - if(textline.contains("File")) { - int iPos = textline.indexOf("=", 0); - ++iPos; - - QString filename = textline.right(textline.length() - iPos); - auto trackFile = Parser::playlistEntryToFileInfo(filename, basePath); - if (trackFile.checkFileExists()) { - return trackFile.location(); - } - // We couldn't match this to a real file so ignore it - qWarning() << trackFile << "not found"; - } - textline = stream->readLine(); - } - - // Signal we reached the end - return QString(); -} - bool ParserPls::writePLSFile(const QString &file_str, const QList &items, bool useRelativePath) { QFile file(file_str); diff --git a/src/library/parserpls.h b/src/library/parserpls.h index eea00b6e857..f43b083cf26 100644 --- a/src/library/parserpls.h +++ b/src/library/parserpls.h @@ -20,11 +20,7 @@ class ParserPls : Parser { public: static bool isPlaylistFilenameSupported(const QString& fileName); - QList parse(const QString& playlistFile, bool keepMissingFiles); + static QList parse(const QString& playlistFile); /// Playlist Export static bool writePLSFile(const QString &file, const QList &items, bool useRelativePath); - - private: - /// Reads a line from the file and returns filepath - static QString getFilePath(QTextStream*, const QString& basePath); }; diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 9f640b589c6..442aea4a7a9 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -440,7 +440,7 @@ void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlist_file) { // folder. We don't need access to this file on a regular basis so we do not // register a security bookmark. - QList locations = Parser::parse(playlist_file, false); + QList locations = Parser::parse(playlist_file); // Iterate over the List that holds locations of playlist entries m_pPlaylistTableModel->addTracks(QModelIndex(), locations); } diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 29cbe5db732..2f44ae686e4 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -606,7 +606,7 @@ void CrateFeature::slotImportPlaylistFile(const QString& playlist_file) { // register a security bookmark. // TODO(XXX): Parsing a list of track locations from a playlist file // is a general task and should be implemented separately. - QList locations = Parser().parse(playlist_file, false); + QList locations = Parser().parse(playlist_file); if (locations.empty()) { return; } diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index e73e6dd9c1b..820a624a84a 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -67,7 +67,7 @@ TEST_F(PlaylistTest, m3uEndOfLine) { m3uFile.write("end.mp3"); m3uFile.close(); - QList entries = ParserM3u().parse(m3uFile.fileName(), true); + QList entries = ParserM3u().parse(m3uFile.fileName()); EXPECT_EQ(entries.size(), 5); if (entries.size() == 5) { EXPECT_TRUE(entries[0].endsWith(QStringLiteral("crlf.mp3"))); diff --git a/src/util/dnd.cpp b/src/util/dnd.cpp index da83dd516b4..10dbef0e30e 100644 --- a/src/util/dnd.cpp +++ b/src/util/dnd.cpp @@ -116,7 +116,7 @@ QList DragAndDropHelper::supportedTracksFromUrls( } if (acceptPlaylists && Parser::isPlaylistFilenameSupported(file)) { - const QList track_list = Parser::parse(file, false); + const QList track_list = Parser::parse(file); for (auto& playlistFile : track_list) { addFileToList(mixxx::FileInfo(playlistFile), &fileInfos); } From 613af64fbd5ef95ded955e14bc0897795a9bfdd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 22:43:43 +0100 Subject: [PATCH 09/21] Fix parsing of line ending in csv and add aworkaround for translated column headers --- src/library/parsercsv.cpp | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index 7ca0466916a..c777a10bfce 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -25,7 +25,7 @@ QList ParserCsv::parse(const QString& playlistFile) { QList > tokens = tokenize(ba, ','); // detect Location column - int loc_col = 0x7fffffff; + int loc_col = -1; if (tokens.size()) { for (int i = 0; i < tokens[0].size(); ++i) { if (tokens[0][i] == QObject::tr("Location")) { @@ -33,13 +33,28 @@ QList ParserCsv::parse(const QString& playlistFile) { break; } } - for (int i = 1; i < tokens.size(); ++i) { - qDebug() << tokens; - if (loc_col < tokens[i].size()) { - qDebug() << tokens[i][loc_col]; - locations.append(tokens[i][loc_col]); + qDebug() << loc_col; + if (loc_col < 0 && tokens.size() > 1) { + // Last resort, find column with path separators + // This happens in case of csv files in a different language + for (int i = 0; i < tokens[1].size(); ++i) { + if (tokens[1][i].contains(QDir::separator())) { + loc_col = i; + break; + } } } + if (loc_col >= 0) { + for (int i = 1; i < tokens.size(); ++i) { + qDebug() << tokens; + if (loc_col < tokens[i].size()) { + locations.append(tokens[i][loc_col]); + } + } + } else { + qInfo() << "No location column found in" + << playlistFile; + } } file.close(); } @@ -62,7 +77,7 @@ QList > ParserCsv::tokenize(const QByteArray& str, char delimiter if (!quotes && c == '"') { quotes = true; } else if (quotes && c== '"' ) { - if (pos + 1 < str.length() && str[pos+1]== '"') { + if (pos + 1 < str.length() && str[pos + 1] == '"') { field.append(c); pos++; } else { @@ -75,6 +90,8 @@ QList > ParserCsv::tokenize(const QByteArray& str, char delimiter tokens[row].append(QString::fromLatin1(field)); } field.clear(); + } else if (!quotes && c == '\r' && str[pos + 1] == '\n') { + // skip \r in \r\n } else if (!quotes && (c == '\r' || c == '\n')) { if (Parser::isUtf8(field.constData())) { tokens[row].append(QString::fromUtf8(field)); From ba37ff7c0bb27ad8c27d65405dc2a409771ac729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 22:54:32 +0100 Subject: [PATCH 10/21] Improve m3uEndOfLine test --- src/test/playlisttest.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index 820a624a84a..a97bf246cdd 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -68,12 +68,10 @@ TEST_F(PlaylistTest, m3uEndOfLine) { m3uFile.close(); QList entries = ParserM3u().parse(m3uFile.fileName()); - EXPECT_EQ(entries.size(), 5); - if (entries.size() == 5) { - EXPECT_TRUE(entries[0].endsWith(QStringLiteral("crlf.mp3"))); - EXPECT_TRUE(entries[1].endsWith(QStringLiteral("cr.mp3"))); - EXPECT_TRUE(entries[2].endsWith(QStringLiteral("lf.mp3"))); - EXPECT_TRUE(entries[3].endsWith(QStringLiteral("EuroSign\u20AC.mp3"))); - EXPECT_TRUE(entries[4].endsWith(QStringLiteral("end.mp3"))); - } + ASSERT_EQ(entries.size(), 5); + EXPECT_TRUE(entries[0].endsWith(QStringLiteral("crlf.mp3"))); + EXPECT_TRUE(entries[1].endsWith(QStringLiteral("cr.mp3"))); + EXPECT_TRUE(entries[2].endsWith(QStringLiteral("lf.mp3"))); + EXPECT_TRUE(entries[3].endsWith(QStringLiteral("EuroSign\u20AC.mp3"))); + EXPECT_TRUE(entries[4].endsWith(QStringLiteral("end.mp3"))); } From 8f3c7dc9fe6a90a5e705581c8596ef4619253d57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 23:09:17 +0100 Subject: [PATCH 11/21] Silence debug --- src/library/parsercsv.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index c777a10bfce..c39181ccc2e 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -33,7 +33,6 @@ QList ParserCsv::parse(const QString& playlistFile) { break; } } - qDebug() << loc_col; if (loc_col < 0 && tokens.size() > 1) { // Last resort, find column with path separators // This happens in case of csv files in a different language @@ -46,7 +45,6 @@ QList ParserCsv::parse(const QString& playlistFile) { } if (loc_col >= 0) { for (int i = 1; i < tokens.size(); ++i) { - qDebug() << tokens; if (loc_col < tokens[i].size()) { locations.append(tokens[i][loc_col]); } From fbcc514db17dc2550489c85687ab2389b316c3d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 29 Nov 2021 23:16:36 +0100 Subject: [PATCH 12/21] Added plsEndOfLine and csvendOfLine tests --- src/test/playlisttest.cpp | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index a97bf246cdd..8325781ee66 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -7,7 +7,9 @@ #include #include "library/parser.h" +#include "library/parsercsv.h" #include "library/parserm3u.h" +#include "library/parserpls.h" class DummyParser : public Parser { public: @@ -75,3 +77,32 @@ TEST_F(PlaylistTest, m3uEndOfLine) { EXPECT_TRUE(entries[3].endsWith(QStringLiteral("EuroSign\u20AC.mp3"))); EXPECT_TRUE(entries[4].endsWith(QStringLiteral("end.mp3"))); } + +TEST_F(PlaylistTest, csvEndOfLine) { + QTemporaryFile csvFile; + ASSERT_TRUE(csvFile.open()); + csvFile.write("#,Location\r\n"); + csvFile.write("1,cr.mp3\r"); + csvFile.write("2,lf.mp3\n"); + csvFile.close(); + + QList entries = ParserCsv().parse(csvFile.fileName()); + ASSERT_EQ(entries.size(), 2); + EXPECT_TRUE(entries[0].endsWith(QStringLiteral("cr.mp3"))); + EXPECT_TRUE(entries[1].endsWith(QStringLiteral("lf.mp3"))); +} + +TEST_F(PlaylistTest, plsEndOfLine) { + QTemporaryFile plsFile; + ASSERT_TRUE(plsFile.open()); + plsFile.write("[playlist]\n"); + plsFile.write("NumberOfEntries=2\r"); + plsFile.write("File0=cr.mp3\r"); + plsFile.write("File1=lf.mp3\n"); + plsFile.close(); + + QList entries = ParserPls().parse(plsFile.fileName()); + ASSERT_EQ(entries.size(), 2); + EXPECT_TRUE(entries[0].endsWith(QStringLiteral("cr.mp3"))); + EXPECT_TRUE(entries[1].endsWith(QStringLiteral("lf.mp3"))); +} From a4eaf99aaa9e385c310dc5d2c548a2b6ea524337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 30 Nov 2021 22:50:53 +0100 Subject: [PATCH 13/21] Removed unsued QString --- src/library/parsercsv.cpp | 1 - src/library/parserpls.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index c39181ccc2e..7c07208a732 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -15,7 +15,6 @@ bool ParserCsv::isPlaylistFilenameSupported(const QString& playlistFile) { // static QList ParserCsv::parse(const QString& playlistFile) { QFile file(playlistFile); - QString basepath = playlistFile.section('/', 0, -2); QList locations; //qDebug() << "ParserCsv: Starting to parse."; diff --git a/src/library/parserpls.cpp b/src/library/parserpls.cpp index c8191235db6..d81ec7ee1ee 100644 --- a/src/library/parserpls.cpp +++ b/src/library/parserpls.cpp @@ -38,7 +38,6 @@ bool ParserPls::isPlaylistFilenameSupported(const QString& playlistFile) { // static QList ParserPls::parse(const QString& playlistFile) { QFile file(playlistFile); - const auto basePath = playlistFile.section('/', 0, -2); QList locations; if (file.open(QIODevice::ReadOnly)) { From a8bbaab525e65f4f612b54a228286796e300e1de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 30 Nov 2021 23:27:42 +0100 Subject: [PATCH 14/21] extract function parseAllLocations() --- src/library/parser.cpp | 26 ++++++++++++++++++-------- src/library/parser.h | 1 + src/library/parsercsv.cpp | 2 +- src/library/parsercsv.h | 2 +- src/library/parserm3u.cpp | 2 +- src/library/parserm3u.h | 2 +- src/library/parserpls.cpp | 2 +- src/library/parserpls.h | 2 +- src/test/playlisttest.cpp | 6 +++--- 9 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/library/parser.cpp b/src/library/parser.cpp index c0fc383ed4d..762ec68272c 100644 --- a/src/library/parser.cpp +++ b/src/library/parser.cpp @@ -18,20 +18,30 @@ bool Parser::isPlaylistFilenameSupported(const QString& playlistFile) { } // static -QList Parser::parse(const QString& playlistFile) { - QList locations; +QList Parser::parseAllLocations(const QString& playlistFile) { if (ParserM3u::isPlaylistFilenameSupported(playlistFile)) { - locations = ParserM3u::parse(playlistFile); - } else if (ParserPls::isPlaylistFilenameSupported(playlistFile)) { - locations = ParserPls::parse(playlistFile); - } else if (ParserCsv::isPlaylistFilenameSupported(playlistFile)) { - locations = ParserCsv::parse(playlistFile); + return ParserM3u::parseAllLocations(playlistFile); + } + + if (ParserPls::isPlaylistFilenameSupported(playlistFile)) { + return ParserPls::parseAllLocations(playlistFile); + } + + if (ParserCsv::isPlaylistFilenameSupported(playlistFile)) { + return ParserCsv::parseAllLocations(playlistFile); } + return QList(); +} + +// static +QList Parser::parse(const QString& playlistFile) { + const QList allLocations = parseAllLocations(playlistFile); + QFileInfo fileInfo(playlistFile); QList existingLocations; - for (auto& location : locations) { + for (const auto& location : allLocations) { mixxx::FileInfo trackFile = Parser::playlistEntryToFileInfo( location, fileInfo.canonicalPath()); if (trackFile.checkFileExists()) { diff --git a/src/library/parser.h b/src/library/parser.h index 98630078eb9..eeb5878462e 100644 --- a/src/library/parser.h +++ b/src/library/parser.h @@ -29,6 +29,7 @@ it afterwards for proper functioning class Parser { public: static bool isPlaylistFilenameSupported(const QString& playlistFile); + static QList parseAllLocations(const QString& playlistFile); static QList parse(const QString& playlistFile); protected: diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index 7c07208a732..4dd9e16a944 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -13,7 +13,7 @@ bool ParserCsv::isPlaylistFilenameSupported(const QString& playlistFile) { } // static -QList ParserCsv::parse(const QString& playlistFile) { +QList ParserCsv::parseAllLocations(const QString& playlistFile) { QFile file(playlistFile); QList locations; diff --git a/src/library/parsercsv.h b/src/library/parsercsv.h index f84818dfb21..1f71013c1c4 100644 --- a/src/library/parsercsv.h +++ b/src/library/parsercsv.h @@ -24,7 +24,7 @@ class ParserCsv : Parser { public: // static static bool isPlaylistFilenameSupported(const QString& playlistFile); - static QList parse(const QString&); + static QList parseAllLocations(const QString&); // Playlist Export static bool writeCSVFile(const QString &file, BaseSqlTableModel* pPlaylistTableModel, bool useRelativePath); // Readable Text export diff --git a/src/library/parserm3u.cpp b/src/library/parserm3u.cpp index 0002492f345..336b9a12550 100644 --- a/src/library/parserm3u.cpp +++ b/src/library/parserm3u.cpp @@ -52,7 +52,7 @@ bool ParserM3u::isPlaylistFilenameSupported(const QString& fileName) { fileName.endsWith(".m3u8", Qt::CaseInsensitive); } -QList ParserM3u::parse(const QString& playlistFile) { +QList ParserM3u::parseAllLocations(const QString& playlistFile) { QList paths; QFile file(playlistFile); diff --git a/src/library/parserm3u.h b/src/library/parserm3u.h index 594d3de0851..d269c5353e0 100644 --- a/src/library/parserm3u.h +++ b/src/library/parserm3u.h @@ -22,7 +22,7 @@ class ParserM3u : Parser { public: static bool isPlaylistFilenameSupported(const QString& fileName); - static QList parse(const QString& playlistFile); + static QList parseAllLocations(const QString& playlistFile); /// Playlist Export static bool writeM3UFile(const QString &file_str, const QList &items, bool useRelativePath, bool useUtf8); static bool writeM3UFile(const QString &file, const QList &items, bool useRelativePath); diff --git a/src/library/parserpls.cpp b/src/library/parserpls.cpp index d81ec7ee1ee..9da4aa03879 100644 --- a/src/library/parserpls.cpp +++ b/src/library/parserpls.cpp @@ -36,7 +36,7 @@ bool ParserPls::isPlaylistFilenameSupported(const QString& playlistFile) { } // static -QList ParserPls::parse(const QString& playlistFile) { +QList ParserPls::parseAllLocations(const QString& playlistFile) { QFile file(playlistFile); QList locations; diff --git a/src/library/parserpls.h b/src/library/parserpls.h index f43b083cf26..b85a06af1b9 100644 --- a/src/library/parserpls.h +++ b/src/library/parserpls.h @@ -20,7 +20,7 @@ class ParserPls : Parser { public: static bool isPlaylistFilenameSupported(const QString& fileName); - static QList parse(const QString& playlistFile); + static QList parseAllLocations(const QString& playlistFile); /// Playlist Export static bool writePLSFile(const QString &file, const QList &items, bool useRelativePath); }; diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index 8325781ee66..45f8a76c0af 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -69,7 +69,7 @@ TEST_F(PlaylistTest, m3uEndOfLine) { m3uFile.write("end.mp3"); m3uFile.close(); - QList entries = ParserM3u().parse(m3uFile.fileName()); + QList entries = ParserM3u().parseAllLocations(m3uFile.fileName()); ASSERT_EQ(entries.size(), 5); EXPECT_TRUE(entries[0].endsWith(QStringLiteral("crlf.mp3"))); EXPECT_TRUE(entries[1].endsWith(QStringLiteral("cr.mp3"))); @@ -86,7 +86,7 @@ TEST_F(PlaylistTest, csvEndOfLine) { csvFile.write("2,lf.mp3\n"); csvFile.close(); - QList entries = ParserCsv().parse(csvFile.fileName()); + QList entries = ParserCsv().parseAllLocations(csvFile.fileName()); ASSERT_EQ(entries.size(), 2); EXPECT_TRUE(entries[0].endsWith(QStringLiteral("cr.mp3"))); EXPECT_TRUE(entries[1].endsWith(QStringLiteral("lf.mp3"))); @@ -101,7 +101,7 @@ TEST_F(PlaylistTest, plsEndOfLine) { plsFile.write("File1=lf.mp3\n"); plsFile.close(); - QList entries = ParserPls().parse(plsFile.fileName()); + QList entries = ParserPls().parseAllLocations(plsFile.fileName()); ASSERT_EQ(entries.size(), 2); EXPECT_TRUE(entries[0].endsWith(QStringLiteral("cr.mp3"))); EXPECT_TRUE(entries[1].endsWith(QStringLiteral("lf.mp3"))); From 9008a9cb8179b6a92a5339815acd07cd74abc626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 5 Dec 2021 15:49:34 +0100 Subject: [PATCH 15/21] rename stream to pStream --- src/library/parserpls.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/library/parserpls.cpp b/src/library/parserpls.cpp index 9da4aa03879..aca876841dd 100644 --- a/src/library/parserpls.cpp +++ b/src/library/parserpls.cpp @@ -10,8 +10,8 @@ namespace { -QString getFilePath(QTextStream* stream) { - QString textline = stream->readLine(); +QString getFilePath(QTextStream* pStream) { + QString textline = pStream->readLine(); while (!textline.isEmpty()) { if (textline.isNull()) { break; @@ -22,7 +22,7 @@ QString getFilePath(QTextStream* stream) { ++iPos; return textline.right(textline.length() - iPos); } - textline = stream->readLine(); + textline = pStream->readLine(); } // Signal we reached the end return QString(); From 5d4d43dfc700d45fe485e894d8be56cb44400230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 5 Dec 2021 22:06:13 +0100 Subject: [PATCH 16/21] make entries const --- src/test/playlisttest.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index 45f8a76c0af..896fef13483 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -69,7 +69,7 @@ TEST_F(PlaylistTest, m3uEndOfLine) { m3uFile.write("end.mp3"); m3uFile.close(); - QList entries = ParserM3u().parseAllLocations(m3uFile.fileName()); + const QList entries = ParserM3u().parseAllLocations(m3uFile.fileName()); ASSERT_EQ(entries.size(), 5); EXPECT_TRUE(entries[0].endsWith(QStringLiteral("crlf.mp3"))); EXPECT_TRUE(entries[1].endsWith(QStringLiteral("cr.mp3"))); @@ -86,7 +86,7 @@ TEST_F(PlaylistTest, csvEndOfLine) { csvFile.write("2,lf.mp3\n"); csvFile.close(); - QList entries = ParserCsv().parseAllLocations(csvFile.fileName()); + const QList entries = ParserCsv().parseAllLocations(csvFile.fileName()); ASSERT_EQ(entries.size(), 2); EXPECT_TRUE(entries[0].endsWith(QStringLiteral("cr.mp3"))); EXPECT_TRUE(entries[1].endsWith(QStringLiteral("lf.mp3"))); @@ -101,7 +101,7 @@ TEST_F(PlaylistTest, plsEndOfLine) { plsFile.write("File1=lf.mp3\n"); plsFile.close(); - QList entries = ParserPls().parseAllLocations(plsFile.fileName()); + const QList entries = ParserPls().parseAllLocations(plsFile.fileName()); ASSERT_EQ(entries.size(), 2); EXPECT_TRUE(entries[0].endsWith(QStringLiteral("cr.mp3"))); EXPECT_TRUE(entries[1].endsWith(QStringLiteral("lf.mp3"))); From 87330f52b66bb04e630aa6bb7444ea47ea8f21d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 5 Dec 2021 22:31:42 +0100 Subject: [PATCH 17/21] prevere at() over operator[] --- src/test/playlisttest.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index 896fef13483..a2ca613a63f 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -71,11 +71,11 @@ TEST_F(PlaylistTest, m3uEndOfLine) { const QList entries = ParserM3u().parseAllLocations(m3uFile.fileName()); ASSERT_EQ(entries.size(), 5); - EXPECT_TRUE(entries[0].endsWith(QStringLiteral("crlf.mp3"))); - EXPECT_TRUE(entries[1].endsWith(QStringLiteral("cr.mp3"))); - EXPECT_TRUE(entries[2].endsWith(QStringLiteral("lf.mp3"))); - EXPECT_TRUE(entries[3].endsWith(QStringLiteral("EuroSign\u20AC.mp3"))); - EXPECT_TRUE(entries[4].endsWith(QStringLiteral("end.mp3"))); + EXPECT_TRUE(entries.at(0).endsWith(QStringLiteral("crlf.mp3"))); + EXPECT_TRUE(entries.at(1).endsWith(QStringLiteral("cr.mp3"))); + EXPECT_TRUE(entries.at(2).endsWith(QStringLiteral("lf.mp3"))); + EXPECT_TRUE(entries.at(3).endsWith(QStringLiteral("EuroSign\u20AC.mp3"))); + EXPECT_TRUE(entries.at(4).endsWith(QStringLiteral("end.mp3"))); } TEST_F(PlaylistTest, csvEndOfLine) { @@ -88,8 +88,8 @@ TEST_F(PlaylistTest, csvEndOfLine) { const QList entries = ParserCsv().parseAllLocations(csvFile.fileName()); ASSERT_EQ(entries.size(), 2); - EXPECT_TRUE(entries[0].endsWith(QStringLiteral("cr.mp3"))); - EXPECT_TRUE(entries[1].endsWith(QStringLiteral("lf.mp3"))); + EXPECT_TRUE(entries.at(0).endsWith(QStringLiteral("cr.mp3"))); + EXPECT_TRUE(entries.at(1).endsWith(QStringLiteral("lf.mp3"))); } TEST_F(PlaylistTest, plsEndOfLine) { @@ -103,6 +103,6 @@ TEST_F(PlaylistTest, plsEndOfLine) { const QList entries = ParserPls().parseAllLocations(plsFile.fileName()); ASSERT_EQ(entries.size(), 2); - EXPECT_TRUE(entries[0].endsWith(QStringLiteral("cr.mp3"))); - EXPECT_TRUE(entries[1].endsWith(QStringLiteral("lf.mp3"))); + EXPECT_TRUE(entries.at(0).endsWith(QStringLiteral("cr.mp3"))); + EXPECT_TRUE(entries.at(1).endsWith(QStringLiteral("lf.mp3"))); } From 317fb656a4f34f564842ffd4ade0e2f2d1d4e03a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 6 Dec 2021 07:38:52 +0100 Subject: [PATCH 18/21] improve variable naming --- src/library/parsercsv.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/library/parsercsv.cpp b/src/library/parsercsv.cpp index 4dd9e16a944..d354dd21182 100644 --- a/src/library/parsercsv.cpp +++ b/src/library/parsercsv.cpp @@ -24,28 +24,28 @@ QList ParserCsv::parseAllLocations(const QString& playlistFile) { QList > tokens = tokenize(ba, ','); // detect Location column - int loc_col = -1; + int locationColumnIndex = -1; if (tokens.size()) { for (int i = 0; i < tokens[0].size(); ++i) { if (tokens[0][i] == QObject::tr("Location")) { - loc_col = i; + locationColumnIndex = i; break; } } - if (loc_col < 0 && tokens.size() > 1) { + if (locationColumnIndex < 0 && tokens.size() > 1) { // Last resort, find column with path separators // This happens in case of csv files in a different language for (int i = 0; i < tokens[1].size(); ++i) { if (tokens[1][i].contains(QDir::separator())) { - loc_col = i; + locationColumnIndex = i; break; } } } - if (loc_col >= 0) { - for (int i = 1; i < tokens.size(); ++i) { - if (loc_col < tokens[i].size()) { - locations.append(tokens[i][loc_col]); + if (locationColumnIndex >= 0) { + for (int row = 1; row < tokens.size(); ++row) { + if (locationColumnIndex < tokens[row].size()) { + locations.append(tokens[row][locationColumnIndex]); } } } else { From db19ac95afc91199c518930a1e87dd76f47bda71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 7 Dec 2021 00:38:49 +0100 Subject: [PATCH 19/21] Inherit Parser public --- src/library/parsercsv.h | 2 +- src/library/parserm3u.h | 2 +- src/library/parserpls.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/library/parsercsv.h b/src/library/parsercsv.h index 1f71013c1c4..4b0b10dcb8c 100644 --- a/src/library/parsercsv.h +++ b/src/library/parsercsv.h @@ -20,7 +20,7 @@ #include "library/parser.h" #include "library/basesqltablemodel.h" -class ParserCsv : Parser { +class ParserCsv : public Parser { public: // static static bool isPlaylistFilenameSupported(const QString& playlistFile); diff --git a/src/library/parserm3u.h b/src/library/parserm3u.h index d269c5353e0..3836ede7f77 100644 --- a/src/library/parserm3u.h +++ b/src/library/parserm3u.h @@ -19,7 +19,7 @@ #include "library/parser.h" -class ParserM3u : Parser { +class ParserM3u : public Parser { public: static bool isPlaylistFilenameSupported(const QString& fileName); static QList parseAllLocations(const QString& playlistFile); diff --git a/src/library/parserpls.h b/src/library/parserpls.h index b85a06af1b9..b19c9b65c15 100644 --- a/src/library/parserpls.h +++ b/src/library/parserpls.h @@ -17,7 +17,7 @@ #include "library/parser.h" -class ParserPls : Parser { +class ParserPls : public Parser { public: static bool isPlaylistFilenameSupported(const QString& fileName); static QList parseAllLocations(const QString& playlistFile); From 3b5d35dd29ae538aed4ab34016adce201585d862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 7 Dec 2021 07:35:07 +0100 Subject: [PATCH 20/21] remove extra space in nested templates --- src/library/parsercsv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/parsercsv.h b/src/library/parsercsv.h index 4b0b10dcb8c..28b2d53d210 100644 --- a/src/library/parsercsv.h +++ b/src/library/parsercsv.h @@ -32,5 +32,5 @@ class ParserCsv : public Parser { private: // Reads a line from the file and returns filepath if a valid file - static QList > tokenize(const QByteArray& str, char delimiter); + static QList> tokenize(const QByteArray& str, char delimiter); }; From 63b49b6a49211cce80fffb55484e436563697781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 9 Dec 2021 14:26:15 +0100 Subject: [PATCH 21/21] Start new test names with a capital --- src/test/playlisttest.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/playlisttest.cpp b/src/test/playlisttest.cpp index a2ca613a63f..9fb6a585d0b 100644 --- a/src/test/playlisttest.cpp +++ b/src/test/playlisttest.cpp @@ -58,7 +58,7 @@ TEST_F(PlaylistTest, Relative) { parser.playlistEntryToFilePath("../../bar.mp3", "base/folder")); } -TEST_F(PlaylistTest, m3uEndOfLine) { +TEST_F(PlaylistTest, M3uEndOfLine) { QTemporaryFile m3uFile; ASSERT_TRUE(m3uFile.open()); m3uFile.write("crlf.mp3\r\n"); @@ -78,7 +78,7 @@ TEST_F(PlaylistTest, m3uEndOfLine) { EXPECT_TRUE(entries.at(4).endsWith(QStringLiteral("end.mp3"))); } -TEST_F(PlaylistTest, csvEndOfLine) { +TEST_F(PlaylistTest, CsvEndOfLine) { QTemporaryFile csvFile; ASSERT_TRUE(csvFile.open()); csvFile.write("#,Location\r\n"); @@ -92,7 +92,7 @@ TEST_F(PlaylistTest, csvEndOfLine) { EXPECT_TRUE(entries.at(1).endsWith(QStringLiteral("lf.mp3"))); } -TEST_F(PlaylistTest, plsEndOfLine) { +TEST_F(PlaylistTest, PlsEndOfLine) { QTemporaryFile plsFile; ASSERT_TRUE(plsFile.open()); plsFile.write("[playlist]\n");