Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix m3u list parsing with /r/n end of line #4547

Merged
merged 22 commits into from
Dec 17, 2021
Merged
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 43 additions & 21 deletions src/library/parser.cpp
Original file line number Diff line number Diff line change
@@ -1,36 +1,57 @@
//
// C++ Implementation: parser
//
// Description: superclass for external formats parsers
//
//
// Author: Ingo Kossyk <[email protected]>, (C) 2004
// Author: Tobias Rafreider [email protected], (C) 2011
//
// Copyright: See COPYING file that comes with this distribution
//
//
#include "library/parser.h"

#include <QtDebug>
#include <QDir>
#include <QFile>
#include <QIODevice>
#include <QUrl>
#include <QtDebug>

#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<QString> Parser::parseAllLocations(const QString& playlistFile) {
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
if (ParserM3u::isPlaylistFilenameSupported(playlistFile)) {
return ParserM3u::parseAllLocations(playlistFile);
}

if (ParserPls::isPlaylistFilenameSupported(playlistFile)) {
return ParserPls::parseAllLocations(playlistFile);
}

if (ParserCsv::isPlaylistFilenameSupported(playlistFile)) {
return ParserCsv::parseAllLocations(playlistFile);
}

void Parser::clearLocations() {
m_sLocations.clear();
return QList<QString>();
}

long Parser::countParsed() {
return (long)m_sLocations.count();
// static
QList<QString> Parser::parse(const QString& playlistFile) {
const QList<QString> allLocations = parseAllLocations(playlistFile);

QFileInfo fileInfo(playlistFile);

QList<QString> existingLocations;
for (const auto& location : allLocations) {
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
@@ -115,6 +136,7 @@ bool Parser::isUtf8(const char* string) {
return true;
}

// static
mixxx::FileInfo Parser::playlistEntryToFileInfo(
const QString& playlistEntry,
const QString& basePath) {
27 changes: 5 additions & 22 deletions src/library/parser.h
Original file line number Diff line number Diff line change
@@ -26,34 +26,17 @@ 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;
/**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<QString> parse(const QString&) = 0;
static bool isPlaylistFilenameSupported(const QString& playlistFile);
static QList<QString> parseAllLocations(const QString& playlistFile);
static QList<QString> parse(const QString& playlistFile);

protected:
// Pointer to the parsed Filelocations
QList<QString> 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());
};
90 changes: 38 additions & 52 deletions src/library/parsercsv.cpp
Original file line number Diff line number Diff line change
@@ -1,77 +1,61 @@
//
// C++ Implementation: parsercsv
//
// Description: module to parse Comma-Separated Values (CSV) formatted playlists (rfc4180)
//
//
// Author: Ingo Kossyk <[email protected]>, (C) 2004
// Author: Tobias Rafreider [email protected], (C) 2011
// Author: Daniel Schürmann [email protected], (C) 2011
//
// Copyright: See COPYING file that comes with this distribution
//
//

#include "library/parsercsv.h"

#include <QDir>
#include <QMessageBox>
#include <QTextStream>
#include <QtDebug>

#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);
}

QList<QString> ParserCsv::parse(const QString& sFilename) {
QFile file(sFilename);
QString basepath = sFilename.section('/', 0, -2);
// static
QList<QString> ParserCsv::parseAllLocations(const QString& playlistFile) {
QFile file(playlistFile);

clearLocations();
QList<QString> locations;
//qDebug() << "ParserCsv: Starting to parse.";
if (file.open(QIODevice::ReadOnly)) {
QByteArray ba = file.readAll();

QList<QList<QString> > tokens = tokenize(ba, ',');

// detect Location column
int loc_coll = 0x7fffffff;
int locationColumnIndex = -1;
if (tokens.size()) {
for (int i = 0; i < tokens[0].size(); ++i) {
if (tokens[0][i] == tr("Location")) {
loc_coll = i;
if (tokens[0][i] == QObject::tr("Location")) {
locationColumnIndex = 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());
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())) {
locationColumnIndex = i;
break;
}
}
}
if (locationColumnIndex >= 0) {
for (int row = 1; row < tokens.size(); ++row) {
if (locationColumnIndex < tokens[row].size()) {
locations.append(tokens[row][locationColumnIndex]);
}
m_sLocations.append(fi.filePath());
}
} else {
qInfo() << "No location column found in"
<< playlistFile;
}
}

file.close();

if (m_sLocations.count() != 0) {
return m_sLocations;
} else {
return QList<QString>(); // NULL pointer returned when no locations were found
}
}

file.close();
return QList<QString>(); //if we get here something went wrong
return locations;
}

// Code was posted at http://www.qtcentre.org/threads/35511-Parsing-CSV-data
@@ -90,21 +74,23 @@ QList<QList<QString> > 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 {
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' && str[pos + 1] == '\n') {
// skip \r in \r\n
} 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));
@@ -130,8 +116,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
@@ -221,8 +207,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;
}

20 changes: 8 additions & 12 deletions src/library/parsercsv.h
Original file line number Diff line number Diff line change
@@ -20,21 +20,17 @@
#include "library/parser.h"
#include "library/basesqltablemodel.h"

class ParserCsv : public Parser
{
Q_OBJECT
public:
ParserCsv();
virtual ~ParserCsv();
/**Overwriting function parse in class Parser**/
QList<QString> parse(const QString&);
class ParserCsv : Parser {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the consequence of leaving out the inheritance visibility specifier? Previously it was inheriting from Parser publicly, now its defaulting to something (I don't know what without having to look it up).
From pure gut feeling, leaving the specifier out seems like bad style to me, though I have 0 idea in practice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It defaults to private.
But to be honest, I have just forget to write it explicitly. I will fix it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that was my concern. Seems like no outside component depends on the inheritance but I think it makes sense for the relationship to be public regardless.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

public:
// static
static bool isPlaylistFilenameSupported(const QString& playlistFile);
static QList<QString> parseAllLocations(const QString&);
// 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<QList<QString> > tokenize(const QByteArray& str, char delimiter);


private:
// Reads a line from the file and returns filepath if a valid file
static QList<QList<QString> > tokenize(const QByteArray& str, char delimiter);
};
Loading