Merge branch '2.3' of github.com:mixxxdj/mixxx

7 files changed, 181 insertions, 74 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c4d6a38d4d..df6024ec0c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -876,6 +876,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
   src/util/dnd.cpp
   src/util/duration.cpp
   src/util/experiment.cpp
+  src/util/file.cpp
   src/util/fileaccess.cpp
   src/util/fileinfo.cpp
   src/util/filename.cpp
diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp
index 64a8f4e704..7fcbd5724c 100644
--- a/src/library/trackset/baseplaylistfeature.cpp
+++ b/src/library/trackset/baseplaylistfeature.cpp
@@ -19,15 +19,20 @@
 #include "moc_baseplaylistfeature.cpp"
 #include "track/track.h"
 #include "util/assert.h"
+#include "util/file.h"
 #include "widget/wlibrary.h"
 #include "widget/wlibrarysidebar.h"
 #include "widget/wlibrarytextbrowser.h"
 
 namespace {
 constexpr QChar kUnsafeFilenameReplacement = '-';
-}
+const ConfigKey kConfigKeyLastImportExportPlaylistDirectory(
+        "[Library]", "LastImportExportPlaylistDirectory");
+
+} // anonymous namespace
 
-BasePlaylistFeature::BasePlaylistFeature(Library* pLibrary,
+BasePlaylistFeature::BasePlaylistFeature(
+        Library* pLibrary,
         UserSettingsPointer pConfig,
         PlaylistTableModel* pModel,
         const QString& rootViewName,
@@ -419,17 +424,18 @@ void BasePlaylistFeature::slotDeletePlaylist() {
 
 void BasePlaylistFeature::slotImportPlaylist() {
     //qDebug() << "slotImportPlaylist() row:" << m_lastRightClickedIndex.data();
-    QString playlist_file = getPlaylistFile();
-    if (playlist_file.isEmpty()) {
+    const QString playlistFile = getPlaylistFile();
+    if (playlistFile.isEmpty()) {
         return;
     }
 
     // Update the import/export playlist directory
-    QFileInfo fileName(playlist_file);
-    m_pConfig->set(ConfigKey("[Library]", "LastImportExportPlaylistDirectory"),
-            ConfigValue(fileName.dir().absolutePath()));
+    QString fileDirectory(playlistFile);
+    fileDirectory.truncate(playlistFile.lastIndexOf(QDir::separator()));
+    m_pConfig->set(kConfigKeyLastImportExportPlaylistDirectory,
+            ConfigValue(fileDirectory));
 
-    slotImportPlaylistFile(playlist_file);
+    slotImportPlaylistFile(playlistFile);
     activateChild(m_lastRightClickedIndex);
 }
 
@@ -446,24 +452,24 @@ void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlist_file) {
 
 void BasePlaylistFeature::slotCreateImportPlaylist() {
     // Get file to read
-    QStringList playlist_files = LibraryFeature::getPlaylistFiles();
-    if (playlist_files.isEmpty()) {
+    const QStringList playlistFiles = LibraryFeature::getPlaylistFiles();
+    if (playlistFiles.isEmpty()) {
         return;
     }
 
     // Set last import directory
-    QFileInfo fileName(playlist_files.first());
-    m_pConfig->set(ConfigKey("[Library]", "LastImportExportPlaylistDirectory"),
-            ConfigValue(fileName.dir().absolutePath()));
+    QString fileDirectory(playlistFiles.first());
+    fileDirectory.truncate(playlistFiles.first().lastIndexOf(QDir::separator()));
+    m_pConfig->set(kConfigKeyLastImportExportPlaylistDirectory,
+            ConfigValue(fileDirectory));
 
     int lastPlaylistId = kInvalidPlaylistId;
 
     // For each selected element create a different playlist.
-    for (const QString& playlistFile : playlist_files) {
-        fileName = QFileInfo(playlistFile);
-
+    for (const QString& playlistFile : playlistFiles) {
+        const QFileInfo fileInfo(playlistFile);
         // Get a valid name
-        QString baseName = fileName.baseName();
+        const QString baseName = fileInfo.baseName();
         QString name;
 
         bool validNameGiven = false;
@@ -508,28 +514,28 @@ void BasePlaylistFeature::slotExportPlaylist() {
     qDebug() << "Export playlist" << playlistName;
 
     QString lastPlaylistDirectory = m_pConfig->getValue(
-            ConfigKey("[Library]", "LastImportExportPlaylistDirectory"),
+            kConfigKeyLastImportExportPlaylistDirectory,
             QStandardPaths::writableLocation(QStandardPaths::MusicLocation));
 
     // Open a dialog to let the user choose the file location for playlist export.
     // The location is set to the last used directory for import/export and the file
     // name to the playlist name.
-    QString filefilter = tr("M3U Playlist (*.m3u)");
-    QString file_location = QFileDialog::getSaveFileName(
-            nullptr,
+    const QString fileLocation = getFilePathWithVerifiedExtensionFromFileDialog(
             tr("Export Playlist"),
-            lastPlaylistDirectory.append("/").append(playlistName),
+            lastPlaylistDirectory.append("/").append(playlistName).append(".m3u"),
             tr("M3U Playlist (*.m3u);;M3U8 Playlist (*.m3u8);;"
                "PLS Playlist (*.pls);;Text CSV (*.csv);;Readable Text (*.txt)"),
-            &filefilter);
-    // Exit method if user cancelled the open dialog.
-    if (file_location.isNull() || file_location.isEmpty()) {
+            tr("M3U Playlist (*.m3u)"));
+    // Exit method if the file name is empty because the user cancelled the save dialog.
+    if (fileLocation.isEmpty()) {
         return;
     }
-    QFileInfo fileName(file_location);
+
     // Update the import/export playlist directory
-    m_pConfig->set(ConfigKey("[Library]", "LastImportExportPlaylistDirectory"),
-            ConfigValue(fileName.dir().absolutePath()));
+    QString fileDirectory(fileLocation);
+    fileDirectory.truncate(fileLocation.lastIndexOf(QDir::separator()));
+    m_pConfig->set(kConfigKeyLastImportExportPlaylistDirectory,
+            ConfigValue(fileDirectory));
 
     // The user has picked a new directory via a file dialog. This means the
     // system sandboxer (if we are sandboxed) has granted us permission to this
@@ -555,28 +561,27 @@ void BasePlaylistFeature::slotExportPlaylist() {
     bool useRelativePath = m_pConfig->getValue<bool>(
             ConfigKey("[Library]", "UseRelativePathOnExport"));
 
-    if (file_location.endsWith(".csv", Qt::CaseInsensitive)) {
-        ParserCsv::writeCSVFile(
-                file_location, pPlaylistTableModel.data(), useRelativePath);
-    } else if (file_location.endsWith(".txt", Qt::CaseInsensitive)) {
+    if (fileLocation.endsWith(".csv", Qt::CaseInsensitive)) {
+        ParserCsv::writeCSVFile(fileLocation, pPlaylistTableModel.data(), useRelativePath);
+    } else if (fileLocation.endsWith(".txt", Qt::CaseInsensitive)) {
         if (m_playlistDao.getHiddenType(pPlaylistTableModel->getPlaylist()) ==
                 PlaylistDAO::PLHT_SET_LOG) {
-            ParserCsv::writeReadableTextFile(
-                    file_location, pPlaylistTableModel.data(), true);
+            ParserCsv::writeReadableTextFile(fileLocation, pPlaylistTableModel.data(), true);
         } else {
-            ParserCsv::writeReadableTextFile(
-                    file_location, pPlaylistTableModel.data(), false);
+            ParserCsv::writeReadableTextFile(fileLocation, pPlaylistTableModel.data(), false);
         }
     } else {
         // Create and populate a list of files of the playlist
-        QList<QString> playlist_items;
+        QList<QString> playlistItems;
         int rows = pPlaylistTableModel->rowCount();
         for (int i = 0; i < rows; ++i) {
             QModelIndex index = pPlaylistTableModel->index(i, 0);
-            playlist_items << pPlaylistTableModel->getTrackLocation(index);
+            playlistItems << pPlaylistTableModel->getTrackLocation(index);
         }
         exportPlaylistItemsIntoFile(
-                file_location, playlist_items, useRelativePath);
+                fileLocation,
+                playlistItems,
+                useRelativePath);
     }
 }
 
diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp
index c99f4ba597..ba84d36afe 100644
--- a/src/library/trackset/crate/cratefeature.cpp
+++ b/src/library/trackset/crate/cratefeature.cpp
@@ -21,6 +21,7 @@
 #include "sources/soundsourceproxy.h"
 #include "track/track.h"
 #include "util/dnd.h"
+#include "util/file.h"
 #include "widget/wlibrary.h"
 #include "widget/wlibrarysidebar.h"
 #include "widget/wlibrarytextbrowser.h"
@@ -36,6 +37,9 @@ QString formatLabel(
                     crateSummary.getTrackDurationText());
 }
 
+const ConfigKey kConfigKeyLastImportExportCrateDirectoryKey(
+        "[Library]", "LastImportExportCrateDirectory");
+
 } // anonymous namespace
 
 CrateFeature::CrateFeature(Library* pLibrary,
@@ -584,28 +588,29 @@ QModelIndex CrateFeature::indexFromCrateId(CrateId crateId) const {
 void CrateFeature::slotImportPlaylist() {
     //qDebug() << "slotImportPlaylist() row:" ; //<< m_lastRightClickedIndex.data();
 
-    QString playlist_file = getPlaylistFile();
-    if (playlist_file.isEmpty()) {
+    QString playlistFile = getPlaylistFile();
+    if (playlistFile.isEmpty()) {
         return;
     }
 
     // Update the import/export crate directory
-    QFileInfo fileName(playlist_file);
-    m_pConfig->set(ConfigKey("[Library]", "LastImportExportCrateDirectory"),
-            ConfigValue(fileName.dir().absolutePath()));
+    QString fileDirectory(playlistFile);
+    fileDirectory.truncate(playlistFile.lastIndexOf(QDir::separator()));
+    m_pConfig->set(kConfigKeyLastImportExportCrateDirectoryKey,
+            ConfigValue(fileDirectory));
 
-    slotImportPlaylistFile(playlist_file);
+    slotImportPlaylistFile(playlistFile);
     activateChild(m_lastRightClickedIndex);
 }
 
-void CrateFeature::slotImportPlaylistFile(const QString& playlist_file) {
+void CrateFeature::slotImportPlaylistFile(const QString& playlistFile) {
     // The user has picked a new directory via a file dialog. This means the
     // system sandboxer (if we are sandboxed) has granted us permission to this
     // folder. We don't need access to this file on a regular basis so we do not
     // 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<QString> locations = Parser().parse(playlist_file);
+    QList<QString> locations = Parser().parse(playlistFile);
     if (locations.empty()) {
         return;
     }
@@ -614,26 +619,27 @@ void CrateFeature::slotImportPlaylistFile(const QString& playlist_file) {
 
 void CrateFeature::slotCreateImportCrate() {
     // Get file to read
-    QStringList playlist_files = LibraryFeature::getPlaylistFiles();
-    if (playlist_files.isEmpty()) {
+    QStringList playlistFiles = LibraryFeature::getPlaylistFiles();
+    if (playlistFiles.isEmpty()) {
         return;
     }
 
     // Set last import directory
-    QFileInfo fileName(playlist_files.first());
-    m_pConfig->set(ConfigKey("[Library]", "LastImportExportCrateDirectory"),
-            ConfigValue(fileName.dir().absolutePath()));
+    QString fileDirectory(playlistFiles.first());
+    fileDirectory.truncate(playlistFiles.first().lastIndexOf(QDir::separator()));
+    m_pConfig->set(kConfigKeyLastImportExportCrateDirectoryKey,
+            ConfigValue(fileDirectory));
 
     CrateId lastCrateId;
 
     // For each selected file
-    for (const QString& playlistFile : playlist_files) {
-        fileName = QFileInfo(playlistFile);
+    for (const QString& playlistFile : playlistFiles) {
+        const QFileInfo fileInfo(playlistFile);
 
         Crate crate;
 
         // Get a valid name
-        QString baseName = fileName.baseName();
+        const QString baseName = fileInfo.baseName();
         for (int i = 0;; ++i) {
             auto name = baseName;
             if (i > 0) {
@@ -695,23 +701,27 @@ void CrateFeature::slotExportPlaylist() {
     }
 
     QString lastCrateDirectory = m_pConfig->getValue(
-            ConfigKey("[Library]", "LastImportExportCrateDirectory"),
+            kConfigKeyLastImportExportCrateDirectoryKey,
             QStandardPaths::writableLocation(QStandardPaths::MusicLocation));
 
-    QString file_location = QFileDialog::getSaveFileName(nullptr,
+    // Open a dialog to let the user choose the file location for crate export.
+    // The location is set to the last used directory for import/export and the file
+    // name to the playlist name.
+    const QString fileLocation = getFilePathWithVerifiedExtensionFromFileDialog(
             tr("Export Crate"),
             lastCrateDirectory.append("/").append(crate.getName()),
             tr("M3U Playlist (*.m3u);;M3U8 Playlist (*.m3u8);;PLS Playlist "
-               "(*.pls);;Text CSV (*.csv);;Readable Text (*.txt)"));
+               "(*.pls);;Text CSV (*.csv);;Readable Text (*.txt)"),
+            tr("M3U Playlist (*.m3u)"));
     // Exit method if user cancelled the open dialog.
-    if (file_location.isNull() || file_location.isEmpty()) {
+    if (fileLocation.isEmpty()) {
         return;
     }
-
     // Update the import/export crate directory
-    QFileInfo fileName(file_location);
-    m_pConfig->set(ConfigKey("[Library]", "LastImportExportCrateDirectory"),
-            ConfigValue(fileName.dir().absolutePath()));
+    QString fileDirectory(fileLocation);
+    fileDirectory.truncate(fileLocation.lastIndexOf(QDir::separator()));
+    m_pConfig->set(kConfigKeyLastImportExportCrateDirectoryKey,
+            ConfigValue(fileDirectory));
 
     // The user has picked a new directory via a file dialog. This means the
     // system sandboxer (if we are sandboxed) has granted us permission to this
@@ -730,21 +740,21 @@ void CrateFeature::slotExportPlaylist() {
     pCrateTableModel->selectCrate(m_crateTableModel.selectedCrate());
     pCrateTableModel->select();
 
-    if (file_location.endsWith(".csv", Qt::CaseInsensitive)) {
-        ParserCsv::writeCSVFile(file_location, pCrateTableModel.data(), useRelativePath);
-    } else if (file_location.endsWith(".txt", Qt::CaseInsensitive)) {
-        ParserCsv::writeReadableTextFile(file_location, pCrateTableModel.data(), false);
+    if (fileLocation.endsWith(".csv", Qt::CaseInsensitive)) {
+        ParserCsv::writeCSVFile(fileLocation, pCrateTableModel.data(), useRelativePath);
+    } else if (fileLocation.endsWith(".txt", Qt::CaseInsensitive)) {
+        ParserCsv::writeReadableTextFile(fileLocation, pCrateTableModel.data(), false);
     } else {
         // populate a list of files of the crate
-        QList<QString> playlist_items;
+        QList<QString> playlistItems;
         int rows = pCrateTableModel->rowCount();
         for (int i = 0; i < rows; ++i) {
             QModelIndex index = m_crateTableModel.index(i, 0);
-            playlist_items << m_crateTableModel.getTrackLocation(index);
+            playlistItems << m_crateTableModel.getTrackLocation(index);
         }
         exportPlaylistItemsIntoFile(
-                file_location,
-                playlist_items,
+                fileLocation,
+                playlistItems,
                 useRelativePath);
     }
 }
diff --git a/src/mixer/baseplayer.h b/src/mixer/baseplayer.h
index ae7905ce95..e6cf1ba102 100644
--- a/src/mixer/baseplayer.h
+++ b/src/mixer/baseplayer.h
@@ -9,7 +9,7 @@ class BasePlayer : public QObject {
     BasePlayer(QObject* pParent, const QString& group);
     ~BasePlayer() override = default;
 
-    inline const QString& getGroup() {
+    inline const QString& getGroup() const {
         return m_group;
     }
 
diff --git a/src/util/file.cpp b/src/util/file.cpp
new file mode 100644
index 0000000000..09de8c8a25
--- /dev/null
+++ b/src/util/file.cpp
@@ -0,0 +1,71 @@
+#include "util/file.h"
+
+#include <QFileDialog>
+#include <QRegularExpression>
+
+namespace {
+
+const QRegularExpression kExtractExtensionRegex(R"(\(\*\.(.*)\)$)");
+
+} //anonymous namespace
+
+QString filePathWithSelectedExtension(const QString& fileLocationInput,
+        const QString& fileFilter) {
+    if (fileLocationInput.isEmpty()) {
+        return {};
+    }
+    QString fileLocation = fileLocationInput;
+    if (fileFilter.isEmpty()) {
+        return fileLocation;
+    }
+
+    // Extract 'ext' from QFileDialog file filter string 'Funky type (*.ext)'
+    const auto extMatch = kExtractExtensionRegex.match(fileFilter);
+    const QString ext = extMatch.captured(1);
+    if (ext.isNull()) {
+        return fileLocation;
+    }
+    const QFileInfo fileName(fileLocation);
+    if (!ext.isEmpty() && fileName.suffix() != ext) {
+        fileLocation.append(".").append(ext);
+    }
+    return fileLocation;
+}
+
+QString getFilePathWithVerifiedExtensionFromFileDialog(
+        const QString& caption,
+        const QString& preSelectedDirectory,
+        const QString& fileFilters,
+        const QString& preSelectedFileFilter) {
+    QString selectedDirectory(preSelectedDirectory);
+    QString selectedFileFilter(preSelectedFileFilter);
+    QString fileLocation;
+
+    while (true) {
+        fileLocation = QFileDialog::getSaveFileName(
+                nullptr,
+                caption,
+                selectedDirectory,
+                fileFilters,
+                &selectedFileFilter);
+        // Exit method if user cancelled the save dialog.
+        if (fileLocation.isEmpty()) {
+            break;
+        }
+        const QString fileLocationAdjusted = filePathWithSelectedExtension(
+                fileLocation,
+                selectedFileFilter);
+        // If the file path has the selected suffix we can assume the user either
+        // selected a new file or already confirmed overwriting an existing file.
+        // Return the file path. Also when the adjusted file path does not exist yet.
+        // Otherwise show the dialog again with the repaired file path pre-selected.
+        if (fileLocation == fileLocationAdjusted ||
+                !QFileInfo::exists(fileLocationAdjusted)) {
+            fileLocation = fileLocationAdjusted;
+            break;
+        } else {
+            selectedDirectory = fileLocationAdjusted;
+        }
+    }
+    return fileLocation;
+}
diff --git a/src/util/file.h b/src/util/file.h
new file mode 100644
index 0000000000..af88b719b0
--- /dev/null
+++ b/src/util/file.h
@@ -0,0 +1,20 @@
+#pragma once
+
+#include <QString>
+
+// Check if the extension from the file filter was added to the file base name.
+// Otherwise add it manually.
+// Works around https://bugreports.qt.io/browse/QTBUG-27186
+QString filePathWithSelectedExtension(const QString& fileLocationInput,
+        const QString& fileFilter);
+
+// Due to Qt bug https://bugreports.qt.io/browse/QTBUG-27186 we may need to
+// manually add the selected extension to the selected file name.
+// Unfortunately, this would bypass Qt's file overwrite dialog. To avoid
+// creating our own file overwrite dialog we show the file dialog again with
+// the repaired file path pre-selected so Qt's overwrite dialog can kick in.
+QString getFilePathWithVerifiedExtensionFromFileDialog(
+        const QString& caption,
+        const QString& preSelectedDirectory,
+        const QString& fileFilters,
+        const QString& preSelectedFileFilter);
diff --git a/src/waveform/waveformwidgetfactory.cpp b/src/waveform/waveformwidgetfactory.cpp
index fd27a5810e..cfad8444fb 100644
--- a/src/waveform/waveformwidgetfactory.cpp
+++ b/src/waveform/waveformwidgetfactory.cpp
@@ -646,7 +646,7 @@ void WaveformWidgetFactory::render() {
                 // Don't bother doing the pre-render work if we aren't going to
                 // render this widget.
                 bool shouldRender = shouldRenderWaveform(pWaveformWidget);
-                shouldRenderWaveforms[i] = shouldRender;
+                shouldRenderWaveforms[static_cast<int>(i)] = shouldRender;
                 if (!shouldRender) {
                     continue;
                 }
@@ -662,7 +662,7 @@ void WaveformWidgetFactory::render() {
                     i < m_waveformWidgetHolders.size();
                     i++) {
                 WaveformWidgetAbstract* pWaveformWidget = m_waveformWidgetHolders[i].m_waveformWidget;
-                if (!shouldRenderWaveforms[i]) {
+                if (!shouldRenderWaveforms[static_cast<int>(i)]) {
                     continue;
                 }
                 pWaveformWidget->render();