Delete Undo Fix Attempt - Should Solve Issue #5701 Using Clementine to delete a song from disk should not be added to the undo stack (#5702)

* Fix delete functionality. Skips to next song if deleted song is also currently playing.

* Added in a bool to determine if the song being deleted is meant to be deleted from the disk or just from the playlist. Took advantage of the unused variable located in removeRows to distinguish between the two options. No longer adds delete from disk to the undo stack, and deleting from the playlist goes to undo stack as it should.

* Added in a bool to determine if the song being deleted is meant to be deleted from the disk or just from the playlist. Took advantage of the unused variable located in removeRows to distinguish between the two options. No longer adds delete from disk to the undo stack, and deleting from the playlist goes to undo stack as it should.

* Update mainwindow.cpp

* Formatting Changes

* Update playlist.cpp

Simplified condition statement.

* Added in a missed boolean for RemoveSelected

* Update playlistview.cpp

src/playlist/playlist.cpp

@@ -1593,6 +1593,8 @@ bool Playlist::removeRows(int row, int count, const QModelIndex& parent) {
     // might have been invalidated.
     RemoveItemsWithoutUndo(row, count);
     undo_stack_->clear();
+  } else if (parent == QModelIndex()) {
+    RemoveItemsWithoutUndo(row, count);
   } else {
     undo_stack_->push(new PlaylistUndoCommands::RemoveItems(this, row, count));
   }

src/playlist/playlistview.cpp

@@ -137,16 +137,14 @@ PlaylistView::PlaylistView(QWidget* parent)
   setStyle(style_);
   setMouseTracking(true);
 
-  QIcon currenttrack_play = IconLoader::Load("currenttrack_play",
-                                             IconLoader::Other);
-  currenttrack_play_ = currenttrack_play.pixmap(currenttrack_play
-                                                .availableSizes()
-                                                .last());
-  QIcon currenttrack_pause = IconLoader::Load("currenttrack_pause",
-                                              IconLoader::Other);
-  currenttrack_pause_ = currenttrack_pause.pixmap(currenttrack_pause
-                                                  .availableSizes()
-                                                  .last());
+  QIcon currenttrack_play =
+      IconLoader::Load("currenttrack_play", IconLoader::Other);
+  currenttrack_play_ =
+      currenttrack_play.pixmap(currenttrack_play.availableSizes().last());
+  QIcon currenttrack_pause =
+      IconLoader::Load("currenttrack_pause", IconLoader::Other);
+  currenttrack_pause_ =
+      currenttrack_pause.pixmap(currenttrack_pause.availableSizes().last());
 
   connect(header_, SIGNAL(sectionResized(int, int, int)), SLOT(SaveGeometry()));
   connect(header_, SIGNAL(sectionMoved(int, int, int)), SLOT(SaveGeometry()));
@@ -600,11 +598,11 @@ void PlaylistView::keyPressEvent(QKeyEvent* event) {
   if (!model() || state() == QAbstractItemView::EditingState) {
     QTreeView::keyPressEvent(event);
   } else if (event == QKeySequence::Delete) {
-    RemoveSelected();
+    RemoveSelected(false);
     event->accept();
 #ifdef Q_OS_DARWIN
   } else if (event->key() == Qt::Key_Backspace) {
-    RemoveSelected();
+    RemoveSelected(false);
     event->accept();
 #endif
   } else if (event == QKeySequence::Copy) {
@@ -643,7 +641,7 @@ void PlaylistView::contextMenuEvent(QContextMenuEvent* e) {
   e->accept();
 }
 
-void PlaylistView::RemoveSelected() {
+void PlaylistView::RemoveSelected(bool deleting_from_disk) {
   int rows_removed = 0;
   QItemSelection selection(selectionModel()->selection());
 
@@ -660,7 +658,12 @@ void PlaylistView::RemoveSelected() {
 
   for (const QItemSelectionRange& range : selection) {
     if (range.top() < last_row) rows_removed += range.height();
-    model()->removeRows(range.top(), range.height(), range.parent());
+
+    if (!deleting_from_disk) {
+      model()->removeRows(range.top(), range.height(), range.topLeft());
+    } else {
+      model()->removeRows(range.top(), range.height(), QModelIndex());
+    }
   }
 
   int new_row = last_row - rows_removed;

src/playlist/playlistview.h

@@ -74,7 +74,7 @@ class PlaylistView : public QTreeView {
   void SetApplication(Application* app);
   void SetItemDelegates(LibraryBackend* backend);
   void SetPlaylist(Playlist* playlist);
-  void RemoveSelected();
+  void RemoveSelected(bool deleting_from_disk);
 
   void SetReadOnlySettings(bool read_only) { read_only_settings_ = read_only; }
 
@@ -246,7 +246,7 @@ signals:
   int drop_indicator_row_;
   bool drag_over_;
 
-  bool ratings_locked_; // To store Ratings section lock status
+  bool ratings_locked_;  // To store Ratings section lock status
 
   DynamicPlaylistControls* dynamic_controls_;
 

src/ui/mainwindow.cpp

@@ -1064,9 +1064,9 @@ void MainWindow::ReloadSettings() {
       AddBehaviour(s.value("doubleclick_addmode", AddBehaviour_Append).toInt());
   doubleclick_playmode_ = PlayBehaviour(
       s.value("doubleclick_playmode", PlayBehaviour_IfStopped).toInt());
-  doubleclick_playlist_addmode_ = PlaylistAddBehaviour(
-      s.value("doubleclick_playlist_addmode", PlaylistAddBehaviour_Play)
-          .toInt());
+  doubleclick_playlist_addmode_ =
+      PlaylistAddBehaviour(s.value("doubleclick_playlist_addmode",
+                                   PlaylistAddBehaviour_Play).toInt());
   menu_playmode_ =
       PlayBehaviour(s.value("menu_playmode", PlayBehaviour_IfStopped).toInt());
 
@@ -2012,9 +2012,9 @@ void MainWindow::AddFile() {
   // Show dialog
   QStringList file_names = QFileDialog::getOpenFileNames(
       this, tr("Add file"), directory,
-      QString("%1 (%2);;%3;;%4")
-          .arg(tr("Music"), FileView::kFileFilter, parser.filters(),
-               tr(kAllFilesFilterSpec)));
+      QString("%1 (%2);;%3;;%4").arg(tr("Music"), FileView::kFileFilter,
+                                     parser.filters(),
+                                     tr(kAllFilesFilterSpec)));
   if (file_names.isEmpty()) return;
 
   // Save last used directory
@@ -2113,7 +2113,7 @@ void MainWindow::ShowInLibrary() {
 }
 
 void MainWindow::PlaylistRemoveCurrent() {
-  ui_->playlist->view()->RemoveSelected();
+  ui_->playlist->view()->RemoveSelected(false);
 }
 
 void MainWindow::PlaylistEditFinished(const QModelIndex& index) {
@@ -2387,7 +2387,20 @@ void MainWindow::PlaylistDelete() {
                           ->Metadata();
   }
 
-  ui_->playlist->view()->RemoveSelected();
+  if (app_->player()->GetState() == Engine::Playing) {
+    if (app_->playlist_manager()->current()->rowCount() ==
+        selected_songs.length()) {
+      app_->player()->Stop();
+    } else {
+      for (Song x : selected_songs) {
+        if (x == app_->player()->GetCurrentItem()->Metadata()) {
+          app_->player()->Next();
+        }
+      }
+    }
+  }
+
+  ui_->playlist->view()->RemoveSelected(true);
 
   DeleteFiles* delete_files = new DeleteFiles(app_->task_manager(), storage);
   connect(delete_files, SIGNAL(Finished(SongList)),