From fccbb98b8e2e5220849cc26da188b9f8b17a18c6 Mon Sep 17 00:00:00 2001 From: Gianluca Recchia Date: Sat, 24 Aug 2019 17:53:11 +0200 Subject: [PATCH] Improve File Dialog * QFileDialog returns UNIX paths, even on Windows. This patch converts what QFileDialog returns to the native path format. * Improve const correctness * Avoid imposing file extension on Linux * This patch improves things like unneeded passes by values, missing const qualifiers, ugly copies because of variable reuse and consistency in variable names. --- src/gui/DatabaseTabWidget.cpp | 13 +- src/gui/DatabaseWidget.cpp | 9 +- src/gui/FileDialog.cpp | 151 ++++++------------ src/gui/FileDialog.h | 24 ++- src/keeshare/SettingsWidgetKeeShare.cpp | 2 +- .../group/EditGroupWidgetKeeShare.cpp | 30 +--- 6 files changed, 70 insertions(+), 159 deletions(-) diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index a56008c7..11b87cad 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -394,8 +394,8 @@ void DatabaseTabWidget::exportToCsv() return; } - QString fileName = fileDialog()->getSaveFileName( - this, tr("Export database to CSV file"), QString(), tr("CSV file").append(" (*.csv)"), nullptr, nullptr, "csv"); + const QString fileName = fileDialog()->getSaveFileName( + this, tr("Export database to CSV file"), QString(), tr("CSV file").append(" (*.csv)"), nullptr, nullptr); if (fileName.isEmpty()) { return; } @@ -419,13 +419,8 @@ void DatabaseTabWidget::exportToHtml() return; } - QString fileName = fileDialog()->getSaveFileName(this, - tr("Export database to HTML file"), - QString(), - tr("HTML file").append(" (*.html)"), - nullptr, - nullptr, - "html"); + const QString fileName = fileDialog()->getSaveFileName( + this, tr("Export database to HTML file"), QString(), tr("HTML file").append(" (*.html)"), nullptr, nullptr); if (fileName.isEmpty()) { return; } diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 31badc58..d695ae2c 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -1656,13 +1656,8 @@ bool DatabaseWidget::saveAs() oldFilePath = QDir::toNativeSeparators(config()->get("LastDir", QDir::homePath()).toString() + "/" + tr("Passwords").append(".kdbx")); } - QString newFilePath = fileDialog()->getSaveFileName(this, - tr("Save database as"), - oldFilePath, - tr("KeePass 2 Database").append(" (*.kdbx)"), - nullptr, - nullptr, - "kdbx"); + const QString newFilePath = fileDialog()->getSaveFileName( + this, tr("Save database as"), oldFilePath, tr("KeePass 2 Database").append(" (*.kdbx)"), nullptr, nullptr); if (!newFilePath.isEmpty()) { // Ensure we don't recurse back into this function diff --git a/src/gui/FileDialog.cpp b/src/gui/FileDialog.cpp index a003bd5b..12f58277 100644 --- a/src/gui/FileDialog.cpp +++ b/src/gui/FileDialog.cpp @@ -19,31 +19,32 @@ #include "core/Config.h" +#include + FileDialog* FileDialog::m_instance(nullptr); QString FileDialog::getOpenFileName(QWidget* parent, const QString& caption, - QString dir, + const QString& dir, const QString& filter, QString* selectedFilter, - QFileDialog::Options options) + const QFileDialog::Options options) { if (!m_nextFileName.isEmpty()) { - QString result = m_nextFileName; + const QString result = m_nextFileName; m_nextFileName.clear(); return result; } else { - if (dir.isEmpty()) { - dir = config()->get("LastDir").toString(); - } - - QString result = QFileDialog::getOpenFileName(parent, caption, dir, filter, selectedFilter, options); + const auto& workingDir = dir.isEmpty() ? config()->get("LastDir").toString() : dir; + const auto result = QDir::toNativeSeparators( + QFileDialog::getOpenFileName(parent, caption, workingDir, filter, selectedFilter, options)); +#ifdef Q_OS_MACOS // on Mac OS X the focus is lost after closing the native dialog if (parent) { parent->activateWindow(); } - +#endif saveLastDir(result); return result; } @@ -51,27 +52,28 @@ QString FileDialog::getOpenFileName(QWidget* parent, QStringList FileDialog::getOpenFileNames(QWidget* parent, const QString& caption, - QString dir, + const QString& dir, const QString& filter, QString* selectedFilter, - QFileDialog::Options options) + const QFileDialog::Options options) { if (!m_nextFileNames.isEmpty()) { - QStringList results = m_nextFileNames; + const QStringList results = m_nextFileNames; m_nextFileNames.clear(); return results; } else { - if (dir.isEmpty()) { - dir = config()->get("LastDir").toString(); - } + const auto& workingDir = dir.isEmpty() ? config()->get("LastDir").toString() : dir; + auto results = QFileDialog::getOpenFileNames(parent, caption, workingDir, filter, selectedFilter, options); - QStringList results = QFileDialog::getOpenFileNames(parent, caption, dir, filter, selectedFilter, options); + for (auto& path : results) + path = QDir::toNativeSeparators(path); +#ifdef Q_OS_MACOS // on Mac OS X the focus is lost after closing the native dialog if (parent) { parent->activateWindow(); } - +#endif if (!results.isEmpty()) { saveLastDir(results[0]); } @@ -81,57 +83,26 @@ QStringList FileDialog::getOpenFileNames(QWidget* parent, QString FileDialog::getFileName(QWidget* parent, const QString& caption, - QString dir, + const QString& dir, const QString& filter, QString* selectedFilter, - QFileDialog::Options options, - const QString& defaultExtension, - const QString& defaultName) + const QFileDialog::Options options) { if (!m_nextFileName.isEmpty()) { - QString result = m_nextFileName; + const QString result = m_nextFileName; m_nextFileName.clear(); return result; } else { - if (dir.isEmpty()) { - dir = config()->get("LastDir").toString(); - } - - QString result; -#if defined(Q_OS_MAC) || defined(Q_OS_WIN) - Q_UNUSED(defaultName); - Q_UNUSED(defaultExtension); - // the native dialogs on these platforms already append the file extension - result = QFileDialog::getSaveFileName(parent, caption, dir, filter, selectedFilter, options); -#else - QFileDialog dialog(parent, caption, dir, filter); - dialog.setFileMode(QFileDialog::AnyFile); - dialog.setAcceptMode(QFileDialog::AcceptSave); - if (selectedFilter) { - dialog.selectNameFilter(*selectedFilter); - } - if (!defaultName.isEmpty()) { - dialog.selectFile(defaultName); - } - dialog.setOptions(options); - if (!defaultExtension.isEmpty()) { - dialog.setDefaultSuffix(defaultExtension); - } - dialog.setLabelText(QFileDialog::Accept, QFileDialog::tr("Select")); - QStringList results; - if (dialog.exec()) { - results = dialog.selectedFiles(); - if (!results.isEmpty()) { - result = results[0]; - } - } -#endif + const auto& workingDir = dir.isEmpty() ? config()->get("LastDir").toString() : dir; + const auto result = QDir::toNativeSeparators( + QFileDialog::getSaveFileName(parent, caption, workingDir, filter, selectedFilter, options)); +#ifdef Q_OS_MACOS // on Mac OS X the focus is lost after closing the native dialog if (parent) { parent->activateWindow(); } - +#endif saveLastDir(result); return result; } @@ -139,81 +110,53 @@ QString FileDialog::getFileName(QWidget* parent, QString FileDialog::getSaveFileName(QWidget* parent, const QString& caption, - QString dir, + const QString& dir, const QString& filter, QString* selectedFilter, - QFileDialog::Options options, - const QString& defaultExtension, - const QString& defaultName) + const QFileDialog::Options options) { if (!m_nextFileName.isEmpty()) { - QString result = m_nextFileName; + const QString result = m_nextFileName; m_nextFileName.clear(); return result; } else { - if (dir.isEmpty()) { - dir = config()->get("LastDir").toString(); - } - - QString result; -#if defined(Q_OS_MACOS) || defined(Q_OS_WIN) - Q_UNUSED(defaultName); - Q_UNUSED(defaultExtension); - // the native dialogs on these platforms already append the file extension - result = QFileDialog::getSaveFileName(parent, caption, dir, filter, selectedFilter, options); -#else - QFileDialog dialog(parent, caption, dir, filter); - dialog.setAcceptMode(QFileDialog::AcceptSave); - dialog.setFileMode(QFileDialog::AnyFile); - if (selectedFilter) { - dialog.selectNameFilter(*selectedFilter); - } - if (!defaultName.isEmpty()) { - dialog.selectFile(defaultName); - } - dialog.setOptions(options); - dialog.setDefaultSuffix(defaultExtension); - - QStringList results; - if (dialog.exec()) { - results = dialog.selectedFiles(); - if (!results.isEmpty()) { - result = results[0]; - } - } -#endif + const auto& workingDir = dir.isEmpty() ? config()->get("LastDir").toString() : dir; + const auto result = QDir::toNativeSeparators( + QFileDialog::getSaveFileName(parent, caption, workingDir, filter, selectedFilter, options)); +#ifdef Q_OS_MACOS // on Mac OS X the focus is lost after closing the native dialog if (parent) { parent->activateWindow(); } - +#endif saveLastDir(result); return result; } } -QString -FileDialog::getExistingDirectory(QWidget* parent, const QString& caption, QString dir, QFileDialog::Options options) +QString FileDialog::getExistingDirectory(QWidget* parent, + const QString& caption, + const QString& dir, + const QFileDialog::Options options) { if (!m_nextDirName.isEmpty()) { - QString result = m_nextDirName; + const QString result = m_nextDirName; m_nextDirName.clear(); return result; } else { - if (dir.isEmpty()) { - dir = config()->get("LastDir").toString(); - } - - dir = QFileDialog::getExistingDirectory(parent, caption, dir, options); + const auto& workingDir = dir.isEmpty() ? config()->get("LastDir").toString() : dir; + const auto result = + QDir::toNativeSeparators(QFileDialog::getExistingDirectory(parent, caption, workingDir, options)); +#ifdef Q_OS_MACOS // on Mac OS X the focus is lost after closing the native dialog if (parent) { parent->activateWindow(); } - - saveLastDir(dir); - return dir; +#endif + saveLastDir(result); + return result; } } diff --git a/src/gui/FileDialog.h b/src/gui/FileDialog.h index eedb691d..7d03d804 100644 --- a/src/gui/FileDialog.h +++ b/src/gui/FileDialog.h @@ -25,40 +25,36 @@ class FileDialog public: QString getOpenFileName(QWidget* parent = nullptr, const QString& caption = QString(), - QString dir = QString(), + const QString& dir = QString(), const QString& filter = QString(), QString* selectedFilter = nullptr, - QFileDialog::Options options = 0); + const QFileDialog::Options options = {}); QStringList getOpenFileNames(QWidget* parent = nullptr, const QString& caption = QString(), - QString dir = QString(), + const QString& dir = QString(), const QString& filter = QString(), QString* selectedFilter = nullptr, - QFileDialog::Options options = 0); + const QFileDialog::Options options = {}); QString getFileName(QWidget* parent = nullptr, const QString& caption = QString(), - QString dir = QString(), + const QString& dir = QString(), const QString& filter = QString(), QString* selectedFilter = nullptr, - QFileDialog::Options options = 0, - const QString& defaultExtension = QString(), - const QString& defaultName = QString()); + const QFileDialog::Options options = {}); QString getSaveFileName(QWidget* parent = nullptr, const QString& caption = QString(), - QString dir = QString(), + const QString& dir = QString(), const QString& filter = QString(), QString* selectedFilter = nullptr, - QFileDialog::Options options = 0, - const QString& defaultExtension = QString(), - const QString& defaultName = QString()); + const QFileDialog::Options options = {}); QString getExistingDirectory(QWidget* parent = nullptr, const QString& caption = QString(), - QString dir = QString(), - QFileDialog::Options options = QFileDialog::ShowDirsOnly); + const QString& dir = QString(), + const QFileDialog::Options options = QFileDialog::ShowDirsOnly); void setNextForgetDialog(); /** diff --git a/src/keeshare/SettingsWidgetKeeShare.cpp b/src/keeshare/SettingsWidgetKeeShare.cpp index 0f33b1df..2ae8b088 100644 --- a/src/keeshare/SettingsWidgetKeeShare.cpp +++ b/src/keeshare/SettingsWidgetKeeShare.cpp @@ -187,7 +187,7 @@ void SettingsWidgetKeeShare::exportCertificate() const auto filters = QString("%1 (*." + filetype + ");;%2 (*)").arg(tr("KeeShare key file"), tr("All files")); QString filename = QString("%1.%2").arg(m_own.certificate.signer).arg(filetype); filename = fileDialog()->getSaveFileName( - this, tr("Select path"), defaultDirPath, filters, nullptr, QFileDialog::Options(0), filetype, filename); + this, tr("Select path"), defaultDirPath, filters, nullptr, QFileDialog::Options(0)); if (filename.isEmpty()) { return; } diff --git a/src/keeshare/group/EditGroupWidgetKeeShare.cpp b/src/keeshare/group/EditGroupWidgetKeeShare.cpp index 0170a82a..5df9f13e 100644 --- a/src/keeshare/group/EditGroupWidgetKeeShare.cpp +++ b/src/keeshare/group/EditGroupWidgetKeeShare.cpp @@ -284,35 +284,17 @@ void EditGroupWidgetKeeShare::launchPathSelectionDialog() } switch (reference.type) { case KeeShareSettings::ImportFrom: - filename = fileDialog()->getFileName(this, - tr("Select import source"), - defaultDirPath, - filters, - nullptr, - QFileDialog::DontConfirmOverwrite, - defaultFiletype, - filename); + filename = fileDialog()->getFileName( + this, tr("Select import source"), defaultDirPath, filters, nullptr, QFileDialog::DontConfirmOverwrite); break; case KeeShareSettings::ExportTo: - filename = fileDialog()->getFileName(this, - tr("Select export target"), - defaultDirPath, - filters, - nullptr, - QFileDialog::Option(0), - defaultFiletype, - filename); + filename = fileDialog()->getFileName( + this, tr("Select export target"), defaultDirPath, filters, nullptr, QFileDialog::Option(0)); break; case KeeShareSettings::SynchronizeWith: case KeeShareSettings::Inactive: - filename = fileDialog()->getFileName(this, - tr("Select import/export file"), - defaultDirPath, - filters, - nullptr, - QFileDialog::Option(0), - defaultFiletype, - filename); + filename = fileDialog()->getFileName( + this, tr("Select import/export file"), defaultDirPath, filters, nullptr, QFileDialog::Option(0)); break; }