Merge pull request #1188 from dogmaphobic/issue1182

File extensions need consistency. Issue #1182.

src/QGCApplication.cc

@@ -484,7 +484,7 @@ void QGCApplication::saveTempFlightDataLogOnMainThread(QString tempLogfile)
         MainWindow::instance(),
         tr("Save Flight Data Log"),
         qgcApp()->mavlinkLogFilesLocation(),
-        tr("Flight Data Log (*.mavlink)"),
+        tr("Flight Data Log Files (*.mavlink)"),
         "mavlink");
     if (!saveFilename.isEmpty()) {
         QFile::copy(tempLogfile, saveFilename);

src/QGCFileDialog.cc

@@ -23,6 +23,7 @@
 
 #include "QGCFileDialog.h"
 #include "QGCApplication.h"
+#include <QRegularExpression>
 #include "MainWindow.h"
 #ifdef QT_DEBUG
 #include "UnitTest.h"
@@ -90,6 +91,7 @@ QString QGCFileDialog::getSaveFileName(
     const QString& dir,
     const QString& filter,
     const QString& defaultSuffix,
+    bool strict,
     Options options)
 {
     _validate(options);
@@ -100,28 +102,104 @@ QString QGCFileDialog::getSaveFileName(
     } else
 #endif
     {
+        QString defaultSuffixCopy(defaultSuffix);
         QFileDialog dlg(parent, caption, dir, filter);
         dlg.setAcceptMode(QFileDialog::AcceptSave);
         if (options) {
             dlg.setOptions(options);
         }
-        if (!defaultSuffix.isEmpty()) {
-            QString suffixCopy(defaultSuffix);
+        if (!defaultSuffixCopy.isEmpty()) {
             //-- Make sure dot is not present
-            if (suffixCopy.startsWith(".")) {
-                suffixCopy.remove(0,1);
+            if (defaultSuffixCopy.startsWith(".")) {
+                defaultSuffixCopy.remove(0,1);
             }
-            dlg.setDefaultSuffix(suffixCopy);
+            dlg.setDefaultSuffix(defaultSuffixCopy);
         }
+        while (true) {
             if (dlg.exec()) {
                 if (dlg.selectedFiles().count()) {
-                return dlg.selectedFiles().first();
+                    QString result = dlg.selectedFiles().first();
+                    //-- If we don't care about the extension, just return it
+                    if (!strict) {
+                        return result;
+                    } else {
+                        //-- We must enforce the file extension
+                        QFileInfo fi(result);
+                        QString userSuffix(fi.suffix());
+                        if (_validateExtension(filter, userSuffix)) {
+                            return result;
                         }
+                        //-- Do we have a default extension?
+                        if (defaultSuffixCopy.isEmpty()) {
+                            //-- We don't, so get the first one in the filter
+                            defaultSuffixCopy = _getFirstExtensionInFilter(filter);
+                        }
+                        //-- If this is set to strict, we have to have a default extension
+                        Q_ASSERT(defaultSuffixCopy.isEmpty() == false);
+                        //-- Forcefully append our desired extension
+                        result += ".";
+                        result += defaultSuffixCopy;
+                        //-- Check and see if this new file already exists
+                        fi.setFile(result);
+                        if (fi.exists()) {
+                            //-- Ask user what to do
+                            QMessageBox msgBox(
+                                QMessageBox::Warning,
+                                tr("File Exists"),
+                                tr("%1 already exists.\nDo you want to replace it?").arg(fi.fileName()),
+                                QMessageBox::Cancel,
+                                parent);
+                            msgBox.addButton(QMessageBox::Retry);
+                            QPushButton *overwriteButton = msgBox.addButton(tr("Replace"), QMessageBox::ActionRole);
+                            msgBox.setDefaultButton(QMessageBox::Retry);
+                            msgBox.setWindowModality(Qt::ApplicationModal);
+                            if (msgBox.exec() == QMessageBox::Retry) {
+                                continue;
+                            } else if (msgBox.clickedButton() == overwriteButton) {
+                                return result;
+                            }
+                        } else {
+                            return result;
+                        }
+                    }
+                }
+            }
+            break;
         }
         return QString("");
     }
 }
 
+/// @brief Make sure filename is using one of the valid extensions defined in the filter
+bool QGCFileDialog::_validateExtension(const QString& filter, const QString& extension) {
+    QRegularExpression re("(\\*\\.\\w+)");
+    QRegularExpressionMatchIterator i = re.globalMatch(filter);
+    while (i.hasNext()) {
+        QRegularExpressionMatch match = i.next();
+        if (match.hasMatch()) {
+            //-- Compare "foo" with "*.foo"
+            if(extension == match.captured(0).mid(2)) {
+                return true;
+            }
+        }
+    }
+    return false;
+}
+
+/// @brief Returns first extension found in filter
+QString QGCFileDialog::_getFirstExtensionInFilter(const QString& filter) {
+    QRegularExpression re("(\\*\\.\\w+)");
+    QRegularExpressionMatchIterator i = re.globalMatch(filter);
+    while (i.hasNext()) {
+        QRegularExpressionMatch match = i.next();
+        if (match.hasMatch()) {
+            //-- Return "foo" from "*.foo"
+            return match.captured(0).mid(2);
+        }
+    }
+    return QString("");
+}
+
 /// @brief Validates and updates the parameters for the file dialog calls
 void QGCFileDialog::_validate(Options& options)
 {

src/QGCFileDialog.h

@@ -108,11 +108,15 @@ public:
         @param[in] dir The initial directory shown to the user.
         @param[in] filter The filter used for selecting the file type.
         @param[in] defaultSuffix Specifies a string that will be added to the filename if it has no suffix already. The suffix is typically used to indicate the file type (e.g. "txt" indicates a text file).
+        @param[in] strict Makes the default suffix mandatory. Only files with those extensions will be allowed.
         @param[in] options Set the various options that affect the look and feel of the dialog.
         @return The full path and filename to be used to save the file or \c QString("") if none.
         @sa <a href="http://qt-project.org/doc/qt-5/qfiledialog.html#getSaveFileName">QFileDialog::getSaveFileName()</a>
         @remark If a default suffix is given, it will be appended to the filename if the user does not enter one themselves. That is, if the user simply enters \e foo and the default suffix is set to \e bar,
-      the returned filename will be \e foo.bar. However, if the user specifies a suffix, none will be added. That is, if the user enters \e foo.txt, that's what you will receive in return.
+        the returned filename will be \e foo.bar. However, if the user specifies a suffix, the \e strict flag will determine what is done. If the user enters \e foo.txt and \e strict is false, the function
+        returns \e foo.txt (no suffix enforced). If \e strict is true however, the default suffix is appended no matter what. In the case above, the function will return \e foo.txt.bar (suffix enforced).
+        @remark If \e strict is set and the file name given by the user is renamed (the \e foo.txt.bar example above), the function will check and see if the file already exists. If that's the case, it will
+        ask the user if they want to overwrite it.
     */
     static QString getSaveFileName(
         QWidget* parent = 0,
@@ -120,6 +124,7 @@ public:
         const QString& dir = QString(),
         const QString& filter = QString(),
         const QString& defaultSuffix = QString(),
+        bool strict = false,
         Options options = 0);
 
 private slots:
@@ -129,6 +134,8 @@ private slots:
     
 private:
     static void    _validate(Options& options);
+    static bool    _validateExtension(const QString& filter, const QString& extension);
+    static QString _getFirstExtensionInFilter(const QString& filter);
 };
 
 

src/ui/MainWindow.cc

@@ -773,17 +773,19 @@ void MainWindow::_createNewCustomWidget(void)
 
 void MainWindow::_loadCustomWidgetFromFile(void)
 {
-    QString widgetFileExtension(".qgw");
-    QString fileName = QGCFileDialog::getOpenFileName(this, tr("Specify Widget File Name"), QStandardPaths::writableLocation(QStandardPaths::DesktopLocation), tr("QGroundControl Widget (*%1);;").arg(widgetFileExtension));
-    if (fileName != "") {
+    QString fileName = QGCFileDialog::getOpenFileName(
+        this, tr("Load Widget File"),
+        QStandardPaths::writableLocation(QStandardPaths::DesktopLocation),
+        tr("QGroundControl Widgets (*.qgw);;All Files (*)"));
+    if (!fileName.isEmpty()) {
         QGCToolWidget* tool = new QGCToolWidget("", "", this);
         if (tool->loadSettings(fileName, true)) {
             QString objectName = tool->objectName() + "DOCK";
-            
             _createDockWidget(tool->getTitle(), objectName, Qt::LeftDockWidgetArea, tool);
             _mapName2DockWidget[objectName]->widget()->setVisible(true);
         }
     }
+    // TODO Add error dialog if widget could not be loaded
 }
 
 void MainWindow::loadSettings()
@@ -849,7 +851,6 @@ void MainWindow::startVideoCapture()
     // TODO: What is this? What kind of "Video" is saved to bmp?
     QString format("bmp");
     QString initialPath = QDir::currentPath() + tr("/untitled.") + format;
-
     QString screenFileName = QGCFileDialog::getSaveFileName(
         this, tr("Save Video Capture"),
         initialPath,

src/ui/QGCBaseParamWidget.cc

@@ -105,15 +105,18 @@ void QGCBaseParamWidget::saveParametersToFile()
 {
     if (!mav)
         return;
-    QString fileName = QGCFileDialog::getSaveFileName(this, tr("Save Parameters"), qgcApp()->savedParameterFilesLocation(), tr("Parameter File (*.txt)"), "txt");
+    QString fileName = QGCFileDialog::getSaveFileName(
+        this, tr("Save Parameters"), qgcApp()->savedParameterFilesLocation(), tr("Parameter Files (*.params)"), "params", true);
+    if (!fileName.isEmpty()) {
         QFile file(fileName);
+        // TODO Display error message to the user if the file can't be created
         if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) {
             return;
         }
-
         QTextStream outstream(&file);
         paramMgr->writeOnboardParamsToStream(outstream,mav->getUASName());
         file.close();
+    }
 }
 
 
@@ -121,15 +124,15 @@ void QGCBaseParamWidget::loadParametersFromFile()
 {
     if (!mav)
         return;
-
-    QString fileName = QGCFileDialog::getOpenFileName(this, tr("Load Parameters"), qgcApp()->savedParameterFilesLocation(), tr("Parameter file (*.txt)"));
+    QString fileName = QGCFileDialog::getOpenFileName(
+        this, tr("Load Parameters"), qgcApp()->savedParameterFilesLocation(),
+        tr("Parameter Files (*.params);;All Files (*)"));
     QFile file(fileName);
-    if (!file.open(QIODevice::ReadOnly | QIODevice::Text))
+    // TODO Display error message to the user if the file can't be opened
+    if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
         return;
-
+    }
     QTextStream in(&file);
     paramMgr->readPendingParamsFromStream(in);
     file.close();
 }
-
-

src/ui/QGCDataPlot2D.cc

@@ -135,7 +135,6 @@ void QGCDataPlot2D::savePlot()
     if (fileName.isEmpty())
         return;
 
-    // TODO This will change once we add "strict" file types in file selection dialogs
     while(!(fileName.endsWith(".svg") || fileName.endsWith(".pdf"))) {
         QMessageBox::StandardButton button = QGCMessageBox::warning(
             tr("Unsuitable file extension for Plot document type."),
@@ -268,16 +267,16 @@ void QGCDataPlot2D::selectFile()
     // Open a file dialog prompting the user for the file to load.
     // Note the special case for the Pixhawk.
     if (ui->inputFileType->currentText().contains("pxIMU") || ui->inputFileType->currentText().contains("RAW")) {
-        fileName = QGCFileDialog::getOpenFileName(this, tr("Specify log file name"), QString(), "Logfile (*.imu *.raw)");
+        fileName = QGCFileDialog::getOpenFileName(this, tr("Load Log File"), QString(), "Log Files (*.imu *.raw)");
     }
     else
     {
-        fileName = QGCFileDialog::getOpenFileName(this, tr("Specify log file name"), QString(), "Logfile (*.csv *.txt *.log)");
+        fileName = QGCFileDialog::getOpenFileName(this, tr("Load Log File"), QString(), "Log Files (*.csv);;All Files (*)");
     }
 
-    // Check if the user hit cancel, which results in a Null string.
+    // Check if the user hit cancel, which results in an empty string.
     // If this is the case, we just stop.
-    if (fileName.isNull())
+    if (fileName.isEmpty())
     {
         return;
     }
@@ -286,9 +285,11 @@ void QGCDataPlot2D::selectFile()
     QFileInfo fileInfo(fileName);
     if (!fileInfo.isReadable())
     {
-        QGCMessageBox::critical(tr("Could not open file"),
+        // TODO This needs some TLC. File used by another program sounds like a Windows only issue.
+        QGCMessageBox::critical(
+            tr("Could not open file"),
             tr("The file is owned by user %1. Is the file currently used by another program?").arg(fileInfo.owner()));
-        ui->filenameLabel->setText(tr("Could not open %1").arg(fileInfo.baseName()+"."+fileInfo.completeSuffix()));
+        ui->filenameLabel->setText(tr("Could not open %1").arg(fileInfo.fileName()));
     }
     else
     {
@@ -696,11 +697,13 @@ void QGCDataPlot2D::saveCsvLog()
 {
     QString fileName = QGCFileDialog::getSaveFileName(
         this, "Save CSV Log File", QStandardPaths::writableLocation(QStandardPaths::DesktopLocation),
-        "CSV file (*.csv);;Text file (*.txt)",
-        "csv");
+        "CSV Files (*.csv)",
+        "csv",
+        true);
 
-    if (fileName.isEmpty())
+    if (fileName.isEmpty()) {
         return; //User cancelled
+    }
 
     bool success = logFile->copy(fileName);
 

src/ui/QGCMAVLinkLogPlayer.cc

@@ -262,10 +262,11 @@ void QGCMAVLinkLogPlayer::_selectLogFileForPlayback(void)
         return;
     }
     
-    QString logFile = QGCFileDialog::getOpenFileName(this,
-                                                    tr("Specify MAVLink log file name to replay"),
+    QString logFile = QGCFileDialog::getOpenFileName(
+        this,
+        tr("Load MAVLink Log File"),
         qgcApp()->mavlinkLogFilesLocation(),
-                                                    tr("MAVLink or Binary Logfile (*.mavlink *.bin *.log)"));
+        tr("MAVLink Log Files (*.mavlink);;All Files (*)"));
 
     if (!logFile.isEmpty()) {
         loadLogFile(logFile);

src/ui/WaypointList.cc

@@ -218,14 +218,14 @@ void WaypointList::setUAS(UASInterface* uas)
 void WaypointList::saveWaypoints()
 {
     // TODO Need better default directory
-    // TODO Need better extension than .txt
-    QString fileName = QGCFileDialog::getSaveFileName(this, tr("Save Waypoint File"), "./waypoints.txt", tr("Waypoint File (*.txt)"), "txt");
+    QString fileName = QGCFileDialog::getSaveFileName(this, tr("Save Waypoint File"), "./untitled.waypoints", tr("Waypoint Files (*.waypoints)"), "waypoints", true);
     WPM->saveWaypoints(fileName);
 }
 
 void WaypointList::loadWaypoints()
 {
-    QString fileName = QGCFileDialog::getOpenFileName(this, tr("Load Waypoint File"), ".", tr("Waypoint File (*.txt)"));
+    // TODO Need better default directory
+    QString fileName = QGCFileDialog::getOpenFileName(this, tr("Load Waypoint File"), ".", tr("Waypoint Files (*.waypoints);;All Files (*)"));
     WPM->loadWaypoints(fileName);
 }
 

src/ui/designer/QGCToolWidget.cc

@@ -571,26 +571,29 @@ void QGCToolWidget::widgetRemoved()
 
 void QGCToolWidget::exportWidget()
 {
-    const QString widgetFileExtension(".qgw");
+    //-- Get file to save
     QString fileName = QGCFileDialog::getSaveFileName(
         this, tr("Save Widget File"),
         QStandardPaths::writableLocation(QStandardPaths::DesktopLocation),
-        tr("QGroundControl Widget (*%1)").arg(widgetFileExtension),
-        "qgw");
-    //-- Note that if the user enters foo.bar, this will end up foo.bar.qgw
-    if (!fileName.endsWith(widgetFileExtension))
-    {
-        fileName = fileName.append(widgetFileExtension);
-    }
+        tr("QGroundControl Widget Files (*.qgw)"),
+        "qgw",
+        true);
+    //-- Save it if we have it
+    if (!fileName.isEmpty()) {
         QSettings settings(fileName, QSettings::IniFormat);
         storeSettings(settings);
+    }
 }
 
 void QGCToolWidget::importWidget()
 {
-    const QString widgetFileExtension(".qgw");
-    QString fileName = QGCFileDialog::getOpenFileName(this, tr("Load Widget File"), QStandardPaths::writableLocation(QStandardPaths::DesktopLocation), tr("QGroundControl Widget (*%1)").arg(widgetFileExtension));
+    QString fileName = QGCFileDialog::getOpenFileName(
+        this, tr("Load Widget File"), QStandardPaths::writableLocation(QStandardPaths::DesktopLocation),
+        tr("QGroundControl Widgets (*.qgw);;All Files (*)"));
+    if (!fileName.isEmpty()) {
+        // TODO There is no error checking. If the load fails, there is nothing telling the user what happened.
         loadSettings(fileName);
+    }
 }
 
 QString QGCToolWidget::getTitle() const

src/ui/linechart/LinechartWidget.cc

@@ -431,48 +431,28 @@ void LinechartWidget::refresh()
     setUpdatesEnabled(true);
 }
 
-QString LinechartWidget::getLogSaveFilename()
-{
-    QString fileName = QGCFileDialog::getSaveFileName(this,
-        tr("Save Log File"),
-        QStandardPaths::writableLocation(QStandardPaths::DesktopLocation),
-        tr("Log file (*.log)"),
-        "log");
-    return fileName;
-}
-
 void LinechartWidget::startLogging()
 {
-    // Store reference to file
-    bool abort = false;
-
     // Check if any curve is enabled
     if (!activePlot->anyCurveVisible()) {
-        QGCMessageBox::critical(tr("No curves selected for logging."), tr("Please check all curves you want to log. Currently no data would be logged, aborting the logging."));
+        QGCMessageBox::critical(
+            tr("No curves selected for logging."),
+            tr("Please check all curves you want to log. Currently no data would be logged. Aborting the logging."));
         return;
     }
 
     // Let user select the log file name
     // QDate date(QDate::currentDate());
     // QString("./pixhawk-log-" + date.toString("yyyy-MM-dd") + "-" + QString::number(logindex) + ".log")
-    QString fileName = getLogSaveFilename();
-
-    while (!(fileName.endsWith(".log")) && !abort && fileName != "") {
-        QMessageBox::StandardButton button = QGCMessageBox::critical(tr("Unsuitable file extension for logfile"),
-                                                                     tr("Please choose .log as file extension. Click OK to change the file extension, cancel to not start logging."),
-                                                                     QMessageBox::Ok | QMessageBox::Cancel,
-                                                                     QMessageBox::Ok);
-        if (button != QMessageBox::Ok) {
-            abort = true;
-            break;
-        }
-        fileName = getLogSaveFilename();
-    }
+    QString fileName = QGCFileDialog::getSaveFileName(this,
+        tr("Save Log File"),
+        QStandardPaths::writableLocation(QStandardPaths::DesktopLocation),
+        tr("Log Files (*.log)"),
+        "log"); // Default type
 
     qDebug() << "SAVE FILE " << fileName;
 
-    // Check if the user did not abort the file save dialog
-    if (!abort && fileName != "") {
+    if (!fileName.isEmpty()) {
         logFile = new QFile(fileName);
         if (logFile->open(QIODevice::Truncate | QIODevice::WriteOnly | QIODevice::Text)) {
             logging = true;
@@ -497,7 +477,8 @@ void LinechartWidget::stopLogging()
         compressor = new LogCompressor(logFile->fileName(), logFile->fileName());
         connect(compressor, SIGNAL(finishedFile(QString)), this, SIGNAL(logfileWritten(QString)));
 
-        QMessageBox::StandardButton button = QGCMessageBox::question(tr("Starting Log Compression"),
+        QMessageBox::StandardButton button = QGCMessageBox::question(
+            tr("Starting Log Compression"),
             tr("Should empty fields (e.g. due to packet drops) be filled with the previous value of the same variable (zero order hold)?"),
             QMessageBox::Yes | QMessageBox::No,
             QMessageBox::No);

src/ui/linechart/LinechartWidget.h

@@ -123,7 +123,6 @@ protected:
     QToolButton* createButton(QWidget* parent);
     void createCurveItem(QString curve);
     void createLayout();
-    QString getLogSaveFilename();
     /** @brief Get the name for a curve key */
     QString getCurveName(const QString& key, bool shortEnabled);
 

src/ui/map3D/Pixhawk3DWidget.cc

@@ -587,11 +587,12 @@ Pixhawk3DWidget::setBirdEyeView(void)
 void
 Pixhawk3DWidget::loadTerrainModel(void)
 {
-    QString filename = QGCFileDialog::getOpenFileName(this, "Load Terrain Model",
+    QString filename = QGCFileDialog::getOpenFileName(
+        this, "Load Terrain Model",
         QStandardPaths::writableLocation(QStandardPaths::DesktopLocation),
-                                                    tr("Collada (*.dae)"));
+        tr("Collada Files (*.dae)"));
 
-    if (filename.isNull())
+    if (filename.isEmpty())
     {
         return;
     }

src/ui/px4_configuration/PX4FirmwareUpgrade.cc

@@ -476,11 +476,11 @@ void PX4FirmwareUpgrade::_getFirmwareFile(void)
     _firmwareFilename = _ui->firmwareCombo->itemData(index).toString();
     Q_ASSERT(!_firmwareFilename.isEmpty());
     if (_firmwareFilename == "selectfile") {
-        _firmwareFilename = QGCFileDialog::getOpenFileName(this,
-                                                    tr("Select Firmware File"),                                             // Dialog title
+        _firmwareFilename = QGCFileDialog::getOpenFileName(
+            this,
+            tr("Load Firmware File"),                                              // Dialog Caption
             QStandardPaths::writableLocation(QStandardPaths::DocumentsLocation),   // Initial directory
             tr("Firmware Files (*.px4 *.bin)"));                                   // File filter
-
     }
     if (!_firmwareFilename.isEmpty()) {
         _downloadFirmware();