| Summary: | Delete key must remove only selected items in saved playlists. | ||
|---|---|---|---|
| Product: | [Applications] amarok | Reporter: | Maxim Dikun <dikmax> |
| Component: | Playlists/Saved Playlists | Assignee: | Amarok Developers <amarok-bugs-dist> |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | bart.cerneels, dikmax |
| Priority: | NOR | Keywords: | release_blocker |
| Version: | 2.3-GIT | ||
| Target Milestone: | 2.2.2 | ||
| Platform: | Ubuntu | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
|
Description
Maxim Dikun
2009-12-13 15:50:13 UTC
Indeed, it asks for confirmation before deleting, and it deletes the last items in the list instead of the selected ones. I mean pressing Delete on concrete items (songs) in playlist. In this case in doesn't asks anything and deletes first item. (In reply to comment #2) > I mean pressing Delete on concrete items (songs) in playlist. > In this case in doesn't asks anything and deletes first item. No, this works well in Amarok 2.2.-git already, it's not working when one selects more than one saved playlist, then it starts removing the latest in the list. commit 484d99240ba76fcf072e94031b88a5eb5996b848 Author: Bart Cerneels <bart.cerneels@kde.org> Date: Tue Dec 22 10:40:14 2009 +0100 Fix removing multiple tracks from a playlist. Also might solve deletion of multiple playlists. BUG:218527 diff --git a/ChangeLog b/ChangeLog index de0942e..f46a6aa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -77,6 +77,8 @@ VERSION 2.2.2 PowerTOP when using the Xine backend. BUGFIXES: + * Fixed removing multiple tracks from a saved playlist only removing the first track. + (BR 218527) * Fixed cancel button not responding when loading thumbnails in the cover manager. (BR 204882) * Fixed crash when using the inline playlist editor to resize items containing diff --git a/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp b/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp index 110a4e7..59217ba 100644 --- a/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp +++ b/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp @@ -42,10 +42,11 @@ bool PlaylistsInGroupsProxy::removeRows( int row, int count, const QModelIndex &parent ) { DEBUG_BLOCK + bool result; debug() << "in parent " << parent << "remove " << count << " starting at row " << row; if( !parent.isValid() ) { - QModelIndex folderIdx = index( row, 0, parent ); + QModelIndex folderIdx = index( row, 0, QModelIndex() ); if( isGroup( folderIdx ) ) { deleteFolder( folderIdx ); @@ -53,27 +54,35 @@ PlaylistsInGroupsProxy::removeRows( int row, int count, const QModelIndex &paren } //is a playlist not in a folder - QModelIndex childIdx = mapToSource( index( row, 0, parent ) ); - return m_model->removeRow( childIdx.row(), QModelIndex() ); + //FIXME: before confirming deletion of a playlist it already dissapears from the + //view. The beginRemoveRows should not be called here but in the source model. + QModelIndex childIdx = mapToSource( index( row, 0, QModelIndex() ) ); + beginRemoveRows( QModelIndex(), row, row + count ); + result = m_model->removeRows( childIdx.row(), count, QModelIndex() ); + endRemoveRows(); + return result; } if( isGroup( parent ) ) { - bool success = true; + result = true; for( int i = row; i < row + count; i++ ) { //individually remove all children of this group in the source model QModelIndex childIdx = mapToSource( index( i, 0, parent ) ); //set success to false if removeRows returns false - success = - m_model->removeRow( childIdx.row(), QModelIndex() ) ? success : false; + result = + m_model->removeRow( childIdx.row(), QModelIndex() ) ? result : false; } - return success; + return result; } //removing a track from a playlist + beginRemoveRows( parent, row, row + count ); QModelIndex originalIdx = mapToSource( parent ); - return m_model->removeRows( row, count, originalIdx ); + result = m_model->removeRows( row, count, originalIdx ); + endRemoveRows(); + return result; } QStringList diff --git a/src/browsers/playlistbrowser/QtGroupingProxy.cpp b/src/browsers/playlistbrowser/QtGroupingProxy.cpp index e5ac775..40b71d6 100755 --- a/src/browsers/playlistbrowser/QtGroupingProxy.cpp +++ b/src/browsers/playlistbrowser/QtGroupingProxy.cpp @@ -85,8 +85,10 @@ QtGroupingProxy::belongsTo( const QModelIndex &idx ) } /* m_groupHash layout -* group index in m_groupMaps: originalRow in m_model of child Playlists (no sub-groups yet) -* group index = -1 for non-grouped playlists +* key : index of the group in m_groupMaps +* values : original row in m_model of children of this group +* +* key = -1 contains non-grouped indexes * * TODO: sub-groups */ @@ -548,7 +550,9 @@ QtGroupingProxy::modelRowsRemoved( const QModelIndex& index, int start, int end Q_UNUSED( index ) Q_UNUSED( start ) Q_UNUSED( end ) - buildTree(); + //don't call buildTree() because it will mess up with removing multiple rows + //individually using removeRow(). + //TODO: this breaks the view when rows are not deleted manually. } void commit 484d99240ba76fcf072e94031b88a5eb5996b848 Author: Bart Cerneels <bart.cerneels@kde.org> Date: Tue Dec 22 10:40:14 2009 +0100 Fix removing multiple tracks from a playlist. Also might solve deletion of multiple playlists. BUG:218527 diff --git a/ChangeLog b/ChangeLog index de0942e..f46a6aa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -77,6 +77,8 @@ VERSION 2.2.2 PowerTOP when using the Xine backend. BUGFIXES: + * Fixed removing multiple tracks from a saved playlist only removing the first track. + (BR 218527) * Fixed cancel button not responding when loading thumbnails in the cover manager. (BR 204882) * Fixed crash when using the inline playlist editor to resize items containing diff --git a/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp b/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp index 110a4e7..59217ba 100644 --- a/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp +++ b/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp @@ -42,10 +42,11 @@ bool PlaylistsInGroupsProxy::removeRows( int row, int count, const QModelIndex &parent ) { DEBUG_BLOCK + bool result; debug() << "in parent " << parent << "remove " << count << " starting at row " << row; if( !parent.isValid() ) { - QModelIndex folderIdx = index( row, 0, parent ); + QModelIndex folderIdx = index( row, 0, QModelIndex() ); if( isGroup( folderIdx ) ) { deleteFolder( folderIdx ); @@ -53,27 +54,35 @@ PlaylistsInGroupsProxy::removeRows( int row, int count, const QModelIndex &paren } //is a playlist not in a folder - QModelIndex childIdx = mapToSource( index( row, 0, parent ) ); - return m_model->removeRow( childIdx.row(), QModelIndex() ); + //FIXME: before confirming deletion of a playlist it already dissapears from the + //view. The beginRemoveRows should not be called here but in the source model. + QModelIndex childIdx = mapToSource( index( row, 0, QModelIndex() ) ); + beginRemoveRows( QModelIndex(), row, row + count ); + result = m_model->removeRows( childIdx.row(), count, QModelIndex() ); + endRemoveRows(); + return result; } if( isGroup( parent ) ) { - bool success = true; + result = true; for( int i = row; i < row + count; i++ ) { //individually remove all children of this group in the source model QModelIndex childIdx = mapToSource( index( i, 0, parent ) ); //set success to false if removeRows returns false - success = - m_model->removeRow( childIdx.row(), QModelIndex() ) ? success : false; + result = + m_model->removeRow( childIdx.row(), QModelIndex() ) ? result : false; } - return success; + return result; } //removing a track from a playlist + beginRemoveRows( parent, row, row + count ); QModelIndex originalIdx = mapToSource( parent ); - return m_model->removeRows( row, count, originalIdx ); + result = m_model->removeRows( row, count, originalIdx ); + endRemoveRows(); + return result; } QStringList diff --git a/src/browsers/playlistbrowser/QtGroupingProxy.cpp b/src/browsers/playlistbrowser/QtGroupingProxy.cpp index e5ac775..40b71d6 100755 --- a/src/browsers/playlistbrowser/QtGroupingProxy.cpp +++ b/src/browsers/playlistbrowser/QtGroupingProxy.cpp @@ -85,8 +85,10 @@ QtGroupingProxy::belongsTo( const QModelIndex &idx ) } /* m_groupHash layout -* group index in m_groupMaps: originalRow in m_model of child Playlists (no sub-groups yet) -* group index = -1 for non-grouped playlists +* key : index of the group in m_groupMaps +* values : original row in m_model of children of this group +* +* key = -1 contains non-grouped indexes * * TODO: sub-groups */ @@ -548,7 +550,9 @@ QtGroupingProxy::modelRowsRemoved( const QModelIndex& index, int start, int end Q_UNUSED( index ) Q_UNUSED( start ) Q_UNUSED( end ) - buildTree(); + //don't call buildTree() because it will mess up with removing multiple rows + //individually using removeRow(). + //TODO: this breaks the view when rows are not deleted manually. } void |