Merge pull request #13610 from ronso0/track-export-skip-fix

(fix) Track file export: various fixes

Author: daschuer

src/library/export/trackexportdlg.cpp

@@ -68,29 +68,30 @@ void TrackExportDlg::slotAskOverwriteMode(
             QMessageBox::Warning,
             tr("Overwrite Existing File?"),
             tr("\"%1\" already exists, overwrite?").arg(filename),
-            QMessageBox::Cancel | QMessageBox::No | QMessageBox::NoToAll
-            | QMessageBox::Yes | QMessageBox::YesToAll);
-    question_box.setDefaultButton(QMessageBox::No);
-    question_box.addButton(tr("&Overwrite"), QMessageBox::YesRole);
-    question_box.addButton(tr("Over&write All"), QMessageBox::YesRole);
-    question_box.addButton(tr("&Skip"), QMessageBox::NoRole);
-    question_box.addButton(tr("Skip &All"), QMessageBox::NoRole);
+            QMessageBox::Cancel);
 
-    switch (question_box.exec()) {
-    case QMessageBox::No:
+    QPushButton* pSkip = question_box.addButton(
+            tr("&Skip"), QMessageBox::NoRole);
+    QPushButton* pSkipAll = question_box.addButton(
+            tr("Skip &All"), QMessageBox::NoRole);
+    QPushButton* pOverwrite = question_box.addButton(
+            tr("&Overwrite"), QMessageBox::YesRole);
+    QPushButton* pOverwriteAll = question_box.addButton(
+            tr("Over&write All"), QMessageBox::YesRole);
+    question_box.setDefaultButton(pSkip);
+
+    question_box.exec();
+    auto* pBtn = question_box.clickedButton();
+    if (pBtn == pSkip) {
         promise->set_value(TrackExportWorker::OverwriteAnswer::SKIP);
-        return;
-    case QMessageBox::NoToAll:
+    } else if (pBtn == pSkipAll) {
         promise->set_value(TrackExportWorker::OverwriteAnswer::SKIP_ALL);
-        return;
-    case QMessageBox::Yes:
+    } else if (pBtn == pOverwrite) {
         promise->set_value(TrackExportWorker::OverwriteAnswer::OVERWRITE);
-        return;
-    case QMessageBox::YesToAll:
+    } else if (pBtn == pOverwriteAll) {
         promise->set_value(TrackExportWorker::OverwriteAnswer::OVERWRITE_ALL);
-        return;
-    case QMessageBox::Cancel:
-    default:
+    } else {
+        // Cancel
         promise->set_value(TrackExportWorker::OverwriteAnswer::CANCEL);
     }
 }

src/library/export/trackexportwizard.cpp

@@ -14,6 +14,11 @@ void TrackExportWizard::exportTracks() {
 }
 
 bool TrackExportWizard::selectDestinationDirectory() {
+    if (m_tracks.isEmpty()) {
+        qInfo() << "TrackExportWizard: No tracks to export, cancel.";
+        return false;
+    }
+
     QString lastExportDirectory = m_pConfig->getValue(
             ConfigKey("[Library]", "LastTrackCopyDirectory"),
             QStandardPaths::writableLocation(QStandardPaths::MusicLocation));

src/library/export/trackexportworker.cpp

@@ -5,9 +5,12 @@
 
 #include "moc_trackexportworker.cpp"
 #include "track/track.h"
+#include "util/logger.h"
 
 namespace {
 
+const mixxx::Logger kLogger("TrackExportWorker");
+
 QString rewriteFilename(const mixxx::FileInfo& fileinfo, int index) {
     // We don't have total control over the inputs, so definitely
     // don't use .arg().arg().arg().
@@ -26,9 +29,12 @@ QMap<QString, mixxx::FileInfo> createCopylist(const TrackPointerList& tracks) {
     // efficiently.
     QMap<QString, mixxx::FileInfo> copylist;
     for (const auto& pTrack : tracks) {
+        VERIFY_OR_DEBUG_ASSERT(pTrack != nullptr) {
+            continue;
+        }
         auto fileInfo = pTrack->getFileInfo();
         if (fileInfo.resolveCanonicalLocation().isEmpty()) {
-            qWarning()
+            kLogger.warning()
                     << "File not found or inaccessible while exporting"
                     << fileInfo;
             // Skip file
@@ -51,7 +57,7 @@ QMap<QString, mixxx::FileInfo> createCopylist(const TrackPointerList& tracks) {
                 break;
             }
             if (++duplicateCounter >= 10000) {
-                qWarning()
+                kLogger.warning()
                         << "Failed to generate a unique file name from"
                         << fileName
                         << "while exporting"
@@ -100,7 +106,7 @@ void TrackExportWorker::copyFile(
             switch (makeOverwriteRequest(dest_path)) {
             case OverwriteAnswer::SKIP:
             case OverwriteAnswer::SKIP_ALL:
-                qDebug() << "skipping" << sourceFilename;
+                kLogger.debug() << "skipping" << sourceFilename;
                 return;
             case OverwriteAnswer::OVERWRITE:
             case OverwriteAnswer::OVERWRITE_ALL:
@@ -112,32 +118,32 @@ void TrackExportWorker::copyFile(
             }
             break;
         case OverwriteMode::SKIP_ALL:
-            qDebug() << "skipping" << sourceFilename;
+            kLogger.debug() << "skipping" << sourceFilename;
             return;
         case OverwriteMode::OVERWRITE_ALL:;
         }
 
         // Remove the existing file in preparation for overwriting.
         QFile dest_file(dest_path);
-        qDebug() << "Removing existing file" << dest_path;
+        kLogger.debug() << "removing existing file" << dest_path;
         if (!dest_file.remove()) {
             const QString error_message = tr(
                     "Error removing file %1: %2. Stopping.").arg(
                     dest_path, dest_file.errorString());
-            qWarning() << error_message;
+            kLogger.warning() << error_message;
             m_errorMessage = error_message;
             stop();
             return;
         }
     }
 
-    qDebug() << "Copying" << sourceFilename << "to" << dest_path;
+    kLogger.debug() << "copying" << sourceFilename << "to" << dest_path;
     QFile source_file(sourceFilename);
     if (!source_file.copy(dest_path)) {
         const QString error_message = tr(
                 "Error exporting track %1 to %2: %3. Stopping.").arg(
                 sourceFilename, dest_path, source_file.errorString());
-        qWarning() << error_message;
+        kLogger.warning() << error_message;
         m_errorMessage = error_message;
         stop();
         return;
@@ -164,7 +170,7 @@ TrackExportWorker::OverwriteAnswer TrackExportWorker::makeOverwriteRequest(
     }
 
     if (!mode_future.valid()) {
-        qWarning() << "TrackExportWorker::makeOverwriteRequest invalid answer from future";
+        kLogger.warning() << "invalid answer from future";
         m_errorMessage = tr("Error exporting tracks");
         stop();
         return OverwriteAnswer::CANCEL;

src/library/trackset/baseplaylistfeature.cpp

@@ -632,7 +632,15 @@ void BasePlaylistFeature::slotExportTrackFiles() {
     TrackPointerList tracks;
     for (int i = 0; i < rows; ++i) {
         QModelIndex index = pPlaylistTableModel->index(i, 0);
-        tracks.push_back(pPlaylistTableModel->getTrack(index));
+        auto pTrack = pPlaylistTableModel->getTrack(index);
+        VERIFY_OR_DEBUG_ASSERT(pTrack != nullptr) {
+            continue;
+        }
+        tracks.push_back(pTrack);
+    }
+
+    if (tracks.isEmpty()) {
+        return;
     }
 
     TrackExportWizard track_export(nullptr, m_pConfig, tracks);

src/library/trackset/crate/cratefeature.cpp

@@ -833,17 +833,29 @@ void CrateFeature::slotExportPlaylist() {
 }
 
 void CrateFeature::slotExportTrackFiles() {
+    CrateId crateId(crateIdFromIndex(m_lastRightClickedIndex));
+    if (!crateId.isValid()) {
+        return;
+    }
     // Create a new table model since the main one might have an active search.
     QScopedPointer<CrateTableModel> pCrateTableModel(
             new CrateTableModel(this, m_pLibrary->trackCollectionManager()));
-    pCrateTableModel->selectCrate(m_crateTableModel.selectedCrate());
+    pCrateTableModel->selectCrate(crateId);
     pCrateTableModel->select();
 
     int rows = pCrateTableModel->rowCount();
     TrackPointerList trackpointers;
     for (int i = 0; i < rows; ++i) {
-        QModelIndex index = m_crateTableModel.index(i, 0);
-        trackpointers.push_back(m_crateTableModel.getTrack(index));
+        QModelIndex index = pCrateTableModel->index(i, 0);
+        auto pTrack = pCrateTableModel->getTrack(index);
+        VERIFY_OR_DEBUG_ASSERT(pTrack != nullptr) {
+            continue;
+        }
+        trackpointers.push_back(pTrack);
+    }
+
+    if (trackpointers.isEmpty()) {
+        return;
     }
 
     TrackExportWizard track_export(nullptr, m_pConfig, trackpointers);