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

WTrackmenu: use moveToTrash() with Qt >= 5.15 #11842

Merged
merged 1 commit into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
75 changes: 70 additions & 5 deletions src/widget/wtrackmenu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,13 @@ void WTrackMenu::createMenus() {
}

if (featureIsEnabled(Feature::RemoveFromDisk)) {
// Qt added QFile::MoveToTrash() in 5.15. If that's not available we
// permanently delete files, put the action into a submenu for safety
// reasons and display different messages in the delete dialogs.
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
m_pRemoveFromDiskMenu = new QMenu(this);
m_pRemoveFromDiskMenu->setTitle(tr("Delete Track Files"));
#endif
}
}

Expand Down Expand Up @@ -274,7 +279,11 @@ void WTrackMenu::createActions() {
}

if (featureIsEnabled(Feature::RemoveFromDisk)) {
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
m_pRemoveFromDiskAct = new QAction(tr("Move Track File(s) to Trash"), this);
#else
m_pRemoveFromDiskAct = new QAction(tr("Delete Files from Disk"), m_pRemoveFromDiskMenu);
#endif
connect(m_pRemoveFromDiskAct,
&QAction::triggered,
this,
Expand Down Expand Up @@ -636,8 +645,12 @@ void WTrackMenu::setupActions() {
}

if (featureIsEnabled(Feature::RemoveFromDisk)) {
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
addAction(m_pRemoveFromDiskAct);
#else
m_pRemoveFromDiskMenu->addAction(m_pRemoveFromDiskAct);
addMenu(m_pRemoveFromDiskMenu);
#endif
}

if (featureIsEnabled(Feature::FileBrowser)) {
Expand Down Expand Up @@ -1964,7 +1977,11 @@ class RemoveTrackFilesFromDiskTrackPointerOperation : public mixxx::TrackPointer
}
QString location = pTrack->getLocation();
QFile file(location);
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
if (file.exists() && !file.moveToTrash()) {
#else
if (file.exists() && !file.remove()) {
#endif
// Deletion failed, log warning and queue location for the
// Failed Deletions warning.
qWarning()
Expand Down Expand Up @@ -2003,8 +2020,8 @@ void WTrackMenu::slotRemoveFromDisk() {
}

{
// Prepare the delete confirmation dialog
// List view for the files to be deleted
// Prepare the delete confirmation dialog.
// First, create the list view for the files to be deleted
// NOTE(ronso0) We could also make this a table to allow showing
// artist and title if file names don't suffice to identify tracks.
QListWidget* delListWidget = new QListWidget();
Expand All @@ -2016,15 +2033,24 @@ void WTrackMenu::slotRemoveFromDisk() {

QString delWarningText;
if (m_pTrackModel) {
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
delWarningText = tr("Permanently delete these files from disk?") +
QStringLiteral("<br><br><b>") +
tr("This can not be undone!") + QStringLiteral("</b>");
} else {
#endif
} else { // track menu of track labels
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)

delWarningText = tr("Stop the deck and move this track file to the trash bin?");
#else
delWarningText =
tr("Stop the deck and permanently delete this track file from disk?") +
QStringLiteral("<br><br><b>") +
tr("This can not be undone!") + QStringLiteral("</b>");
#endif
}

// Setup the warning message and dialog buttons
QLabel* delWarning = new QLabel();
delWarning->setText(delWarningText);
delWarning->setTextFormat(Qt::RichText);
Expand All @@ -2036,7 +2062,11 @@ void WTrackMenu::slotRemoveFromDisk() {
tr("Cancel"),
QDialogButtonBox::RejectRole);
QPushButton* deleteBtn = delButtons->addButton(
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
tr("Delete Files"),
#else
tr("Okay"),
#endif
QDialogButtonBox::AcceptRole);
cancelBtn->setDefault(true);

Expand All @@ -2046,9 +2076,14 @@ void WTrackMenu::slotRemoveFromDisk() {
delLayout->addWidget(delWarning);
delLayout->addWidget(delButtons);

// Create and populate the dialog
QDialog dlgDelConfirm;
dlgDelConfirm.setModal(true); // just to be sure
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
dlgDelConfirm.setWindowTitle(tr("Delete Track Files"));
#else
dlgDelConfirm.setWindowTitle(tr("Move Track File(s) to Trash?"));
#endif
// This is required after customizing the buttons, otherwise neither button
// would close the dialog.
connect(cancelBtn, &QPushButton::clicked, &dlgDelConfirm, &QDialog::reject);
Expand All @@ -2062,14 +2097,20 @@ void WTrackMenu::slotRemoveFromDisk() {

// If the operation was initiated from a deck's track menu
// we'll first stop the deck and eject the track.
// TODO(ronso0) Consider querying PlayerManager if any of the tracks is loaded
// into a (playing?) deck?
if (m_pTrack) {
ControlObject::set(ConfigKey(m_deckGroup, "stop"), 1.0);
ControlObject::set(ConfigKey(m_deckGroup, "eject"), 1.0);
}

// Set up and initiate the track batch operation
const auto progressLabelText =
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
tr("Removing %n track file(s) from disk...",
#else
tr("Moving %n track file(s) to trash...",
#endif
"",
getTrackCount());
const auto trackOperator =
Expand All @@ -2090,19 +2131,35 @@ void WTrackMenu::slotRemoveFromDisk() {
QString msgTitle;
QString msgText;
if (m_pTrackModel) {
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
msgTitle = tr("Track Files Deleted");
#else
msgTitle = tr("Track Files Moved To Trash");
#endif
msgText =
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
tr("%1 track files were moved to trash and purged "
"from the Mixxx database.")
#else
tr("%1 track files were deleted from disk and purged "
Comment on lines +2141 to 2144
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this also follow the dynamic plural forms?

Suggested change
tr("%1 track files were moved to trash and purged "
"from the Mixxx database.")
#else
tr("%1 track files were deleted from disk and purged "
tr("%n track file(s) were moved to trash and purged "
"from the Mixxx database.")
#else
tr("%n track file(s) were deleted from disk and purged "

Copy link
Member Author

Choose a reason for hiding this comment

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

I just didn't want to touch existing tr strings to not lose the translations short before the release. Considering we drop support for Ubuntu 20.04 and can also switch to Qt 5.15 on macOS for Mixxx 2.5, I think the tr inconsistencies are okay.

Copy link
Member

Choose a reason for hiding this comment

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

fair.

"from the Mixxx database.")
#endif
.arg(QString::number(tracksToPurge.length())) +
QStringLiteral("<br><br>") +
tr("Note: if you are in Browse or Recording you need to "
"click the current view again to see changes.");
tr("Note: if you are in the Computer or Recording view you "
"need to click the current view again to see changes.");
} else {
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
msgTitle = tr("Track File Moved To Trash");
msgText = tr(
"Track file was moved to trash and purged "
"from the Mixxx database.");
#else
msgTitle = tr("Track File Deleted");
msgText = tr(
"Track file was deleted from disk and purged "
"from the Mixxx database.");
#endif
}
msgBoxPurgeTracks.setWindowTitle(msgTitle);
msgBoxPurgeTracks.setText(msgText);
Expand All @@ -2122,11 +2179,19 @@ void WTrackMenu::slotRemoveFromDisk() {
QString msgText;
if (m_pTrackModel) {
msgText =
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
tr("The following %1 file(s) could not be moved to trash")
#else
tr("The following %1 file(s) could not be deleted from disk")
#endif
.arg(QString::number(
tracksToKeep.length()));
} else {
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
msgText = tr("This track file could not be moved to trash");
#else
msgText = tr("This track file could not be deleted from disk");
#endif
}
notDeletedLabel->setText(msgText);
notDeletedLabel->setTextFormat(Qt::RichText);
Expand Down
2 changes: 2 additions & 0 deletions src/widget/wtrackmenu.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,9 @@ class WTrackMenu : public QMenu {
WCoverArtMenu* m_pCoverMenu{};
parented_ptr<WSearchRelatedTracksMenu> m_pSearchRelatedMenu;
parented_ptr<WFindOnWebMenu> m_pFindOnWebMenu;
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
QMenu* m_pRemoveFromDiskMenu{};
#endif

// Update ReplayGain from Track
QAction* m_pUpdateReplayGainAct{};
Expand Down