Merge pull request #17392 from Chocobo1/validatePath

Fix path validator

src/gui/fspathedit.cpp

@@ -70,14 +70,14 @@ class FileSystemPathEdit::FileSystemPathEditPrivate
     Q_DECLARE_PUBLIC(FileSystemPathEdit)
     Q_DISABLE_COPY_MOVE(FileSystemPathEditPrivate)
 
-    FileSystemPathEditPrivate(FileSystemPathEdit *q, Private::FileEditorWithCompletion *editor);
+    FileSystemPathEditPrivate(FileSystemPathEdit *q, Private::IFileEditorWithCompletion *editor);
 
     void modeChanged();
     void browseActionTriggered();
     QString dialogCaptionOrDefault() const;
 
     FileSystemPathEdit *q_ptr = nullptr;
-    std::unique_ptr<Private::FileEditorWithCompletion> m_editor;
+    std::unique_ptr<Private::IFileEditorWithCompletion> m_editor;
     QAction *m_browseAction = nullptr;
     QToolButton *m_browseBtn = nullptr;
     QString m_fileNameFilter;
@@ -88,7 +88,7 @@ class FileSystemPathEdit::FileSystemPathEditPrivate
 };
 
 FileSystemPathEdit::FileSystemPathEditPrivate::FileSystemPathEditPrivate(
-                        FileSystemPathEdit *q, Private::FileEditorWithCompletion *editor)
+                        FileSystemPathEdit *q, Private::IFileEditorWithCompletion *editor)
     : q_ptr {q}
     , m_editor {editor}
     , m_browseAction {new QAction(q)}
@@ -183,13 +183,12 @@ void FileSystemPathEdit::FileSystemPathEditPrivate::modeChanged()
     m_validator->setCheckWritePermission((m_mode == FileSystemPathEdit::Mode::FileSave) || (m_mode == FileSystemPathEdit::Mode::DirectorySave));
 }
 
-FileSystemPathEdit::FileSystemPathEdit(Private::FileEditorWithCompletion *editor, QWidget *parent)
+FileSystemPathEdit::FileSystemPathEdit(Private::IFileEditorWithCompletion *editor, QWidget *parent)
     : QWidget(parent)
     , d_ptr(new FileSystemPathEditPrivate(this, editor))
 {
     Q_D(FileSystemPathEdit);
     editor->widget()->setParent(this);
-    setFocusProxy(editor->widget());
 
     auto *layout = new QHBoxLayout(this);
     layout->setContentsMargins(0, 0, 0, 0);

src/gui/fspathedit.h

@@ -35,8 +35,8 @@
 namespace Private
 {
     class FileComboEdit;
-    class FileEditorWithCompletion;
     class FileLineEdit;
+    class IFileEditorWithCompletion;
 }
 
 /*!
@@ -92,7 +92,7 @@ signals:
     void selectedPathChanged(const Path &path);
 
 protected:
-    explicit FileSystemPathEdit(Private::FileEditorWithCompletion *editor, QWidget *parent);
+    explicit FileSystemPathEdit(Private::IFileEditorWithCompletion *editor, QWidget *parent);
 
     template <class Widget>
     Widget *editWidget() const

src/gui/fspathedit_p.cpp

@@ -1,5 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
+ * Copyright (C) 2022 Mike Tzou (Chocobo1)
  * Copyright (C) 2016 Eugene Shalygin
  *
  * This program is free software; you can redistribute it and/or
@@ -42,13 +43,6 @@
 // -------------------- FileSystemPathValidator ----------------------------------------
 Private::FileSystemPathValidator::FileSystemPathValidator(QObject *parent)
     : QValidator(parent)
-    , m_strictMode(false)
-    , m_existingOnly(false)
-    , m_directoriesOnly(false)
-    , m_checkReadPermission(false)
-    , m_checkWritePermission(false)
-    , m_lastTestResult(TestResult::DoesNotExist)
-    , m_lastValidationState(QValidator::Invalid)
 {
 }
 
@@ -57,9 +51,9 @@ bool Private::FileSystemPathValidator::strictMode() const
     return m_strictMode;
 }
 
-void Private::FileSystemPathValidator::setStrictMode(bool v)
+void Private::FileSystemPathValidator::setStrictMode(const bool value)
 {
-    m_strictMode = v;
+    m_strictMode = value;
 }
 
 bool Private::FileSystemPathValidator::existingOnly() const
@@ -67,9 +61,9 @@ bool Private::FileSystemPathValidator::existingOnly() const
     return m_existingOnly;
 }
 
-void Private::FileSystemPathValidator::setExistingOnly(bool v)
+void Private::FileSystemPathValidator::setExistingOnly(const bool value)
 {
-    m_existingOnly = v;
+    m_existingOnly = value;
 }
 
 bool Private::FileSystemPathValidator::directoriesOnly() const
@@ -77,9 +71,9 @@ bool Private::FileSystemPathValidator::directoriesOnly() const
     return m_directoriesOnly;
 }
 
-void Private::FileSystemPathValidator::setDirectoriesOnly(bool v)
+void Private::FileSystemPathValidator::setDirectoriesOnly(const bool value)
 {
-    m_directoriesOnly = v;
+    m_directoriesOnly = value;
 }
 
 bool Private::FileSystemPathValidator::checkReadPermission() const
@@ -87,9 +81,9 @@ bool Private::FileSystemPathValidator::checkReadPermission() const
     return m_checkReadPermission;
 }
 
-void Private::FileSystemPathValidator::setCheckReadPermission(bool v)
+void Private::FileSystemPathValidator::setCheckReadPermission(const bool value)
 {
-    m_checkReadPermission = v;
+    m_checkReadPermission = value;
 }
 
 bool Private::FileSystemPathValidator::checkWritePermission() const
@@ -97,92 +91,37 @@ bool Private::FileSystemPathValidator::checkWritePermission() const
     return m_checkWritePermission;
 }
 
-void Private::FileSystemPathValidator::setCheckWritePermission(bool v)
+void Private::FileSystemPathValidator::setCheckWritePermission(const bool value)
 {
-    m_checkWritePermission = v;
+    m_checkWritePermission = value;
 }
 
-QValidator::State Private::FileSystemPathValidator::validate(QString &input, int &pos) const
-{
-    if (input.isEmpty())
-        return m_strictMode ? QValidator::Invalid : QValidator::Intermediate;
-
-    // we test path components from beginning to the one with cursor location in strict mode
-    // and the one with cursor and beyond in non-strict mode
-    QList<QStringView> components = QStringView(input).split(QDir::separator(), Qt::KeepEmptyParts);
-    // find index of the component that contains pos
-    int componentWithCursorIndex = 0;
-    int componentWithCursorPosition = 0;
-    int pathLength = 0;
-
-    // components.size() - 1 because when path ends with QDir::separator(), we will not see the last
-    // character in the components array, yet everything past the one before the last delimiter
-    // belongs to the last component
-    for (; (componentWithCursorIndex < (components.size() - 1)) && (pathLength < pos); ++componentWithCursorIndex)
-    {
-        pathLength = componentWithCursorPosition + components[componentWithCursorIndex].size();
-        componentWithCursorPosition += components[componentWithCursorIndex].size() + 1;
-    }
-
-    Q_ASSERT(componentWithCursorIndex < components.size());
-
-    m_lastValidationState = QValidator::Acceptable;
-    if (componentWithCursorIndex > 0)
-        m_lastValidationState = validate(components, m_strictMode, 0, componentWithCursorIndex - 1);
-    if ((m_lastValidationState == QValidator::Acceptable) && (componentWithCursorIndex < components.size()))
-        m_lastValidationState = validate(components, false, componentWithCursorIndex, components.size() - 1);
-    return m_lastValidationState;
-}
-
-QValidator::State Private::FileSystemPathValidator::validate(const QList<QStringView> &pathComponents, bool strict,
-                                                             int firstComponentToTest, int lastComponentToTest) const
+Private::FileSystemPathValidator::TestResult
+Private::FileSystemPathValidator::testPath(const Path &path) const
 {
-    Q_ASSERT(firstComponentToTest >= 0);
-    Q_ASSERT(lastComponentToTest >= firstComponentToTest);
-    Q_ASSERT(lastComponentToTest < pathComponents.size());
+    // `QFileInfo` will cache the query results and avoid exessive querying to filesystem
+    const QFileInfo info {path.data()};
 
-    m_lastTestResult = TestResult::DoesNotExist;
-    if (pathComponents.empty())
-        return strict ? QValidator::Invalid : QValidator::Intermediate;
+    if (existingOnly() && !info.exists())
+        return TestResult::DoesNotExist;
 
-    for (int i = firstComponentToTest; i <= lastComponentToTest; ++i)
+    if (directoriesOnly())
     {
-        const bool isFinalPath = (i == (pathComponents.size() - 1));
-        const QStringView componentPath = pathComponents[i];
-        if (componentPath.isEmpty()) continue;
-
-        m_lastTestResult = testPath(Path(pathComponents[i].toString()), isFinalPath);
-        if (m_lastTestResult != TestResult::OK)
-        {
-            m_lastTestedPath = componentPath.toString();
-            return strict ? QValidator::Invalid : QValidator::Intermediate;
-        }
+        if (!info.isDir())
+            return TestResult::NotADir;
     }
-
-    return QValidator::Acceptable;
-}
-
-Private::FileSystemPathValidator::TestResult
-Private::FileSystemPathValidator::testPath(const Path &path, bool pathIsComplete) const
-{
-    QFileInfo fi {path.data()};
-    if (m_existingOnly && !fi.exists())
-        return TestResult::DoesNotExist;
-
-    if ((!pathIsComplete || m_directoriesOnly) && !fi.isDir())
-        return TestResult::NotADir;
-
-    if (pathIsComplete)
+    else
     {
-        if (!m_directoriesOnly && fi.isDir())
+        if (!info.isFile())
             return TestResult::NotAFile;
-
-        if (m_checkWritePermission && (fi.exists() && !fi.isWritable()))
-            return TestResult::CantWrite;
-        if (m_checkReadPermission && !fi.isReadable())
-            return TestResult::CantRead;
     }
 
+    if (checkReadPermission() && !info.isReadable())
+        return TestResult::CantRead;
+
+    if (checkWritePermission() && !info.isWritable())
+        return TestResult::CantWrite;
+
     return TestResult::OK;
 }
 
@@ -196,9 +135,17 @@ QValidator::State Private::FileSystemPathValidator::lastValidationState() const
     return m_lastValidationState;
 }
 
-QString Private::FileSystemPathValidator::lastTestedPath() const
+QValidator::State Private::FileSystemPathValidator::validate(QString &input, int &pos) const
 {
-    return m_lastTestedPath;
+    // ignore cursor position and validate the full path anyway
+    Q_UNUSED(pos);
+
+    m_lastTestResult = testPath(Path(input));
+    m_lastValidationState = (m_lastTestResult == TestResult::OK)
+        ? QValidator::Acceptable
+        : (strictMode() ? QValidator::Invalid : QValidator::Intermediate);
+
+    return m_lastValidationState;
 }
 
 Private::FileLineEdit::FileLineEdit(QWidget *parent)
@@ -208,11 +155,15 @@ Private::FileLineEdit::FileLineEdit(QWidget *parent)
     , m_browseAction {nullptr}
     , m_warningAction {nullptr}
 {
-    m_completerModel->setRootPath({});
+    m_iconProvider.setOptions(QFileIconProvider::DontUseCustomDirectoryIcons);
+
     m_completerModel->setIconProvider(&m_iconProvider);
+    m_completerModel->setOptions(QFileSystemModel::DontWatchForChanges);
+
     m_completer->setModel(m_completerModel);
-    m_completer->setCompletionMode(QCompleter::PopupCompletion);
     setCompleter(m_completer);
+
+    connect(this, &QLineEdit::textChanged, this, &FileLineEdit::validateText);
 }
 
 Private::FileLineEdit::~FileLineEdit()
@@ -220,10 +171,10 @@ Private::FileLineEdit::~FileLineEdit()
     delete m_completerModel; // has to be deleted before deleting the m_iconProvider object
 }
 
-void Private::FileLineEdit::completeDirectoriesOnly(bool completeDirsOnly)
+void Private::FileLineEdit::completeDirectoriesOnly(const bool completeDirsOnly)
 {
-    QDir::Filters filters = completeDirsOnly ? QDir::Dirs : QDir::AllEntries;
-    filters |= QDir::NoDotAndDotDot;
+    const QDir::Filters filters = QDir::NoDotAndDotDot
+        | (completeDirsOnly ? QDir::Dirs : QDir::AllEntries);
     m_completerModel->setFilter(filters);
 }
 
@@ -260,40 +211,12 @@ QWidget *Private::FileLineEdit::widget()
 void Private::FileLineEdit::keyPressEvent(QKeyEvent *e)
 {
     QLineEdit::keyPressEvent(e);
+
     if ((e->key() == Qt::Key_Space) && (e->modifiers() == Qt::CTRL))
     {
-        m_completerModel->setRootPath(QFileInfo(text()).absoluteDir().absolutePath());
+        m_completerModel->setRootPath(Path(text()).data());
         showCompletionPopup();
     }
-
-    auto *validator = qobject_cast<const FileSystemPathValidator *>(this->validator());
-    if (validator)
-    {
-        FileSystemPathValidator::TestResult lastTestResult = validator->lastTestResult();
-        QValidator::State lastState = validator->lastValidationState();
-        if (lastTestResult == FileSystemPathValidator::TestResult::OK)
-        {
-            delete m_warningAction;
-            m_warningAction = nullptr;
-        }
-        else
-        {
-            if (!m_warningAction)
-            {
-                m_warningAction = new QAction(this);
-                addAction(m_warningAction, QLineEdit::TrailingPosition);
-            }
-        }
-
-        if (m_warningAction)
-        {
-            if (lastState == QValidator::Invalid)
-                m_warningAction->setIcon(style()->standardIcon(QStyle::SP_MessageBoxCritical));
-            else if (lastState == QValidator::Intermediate)
-                m_warningAction->setIcon(style()->standardIcon(QStyle::SP_MessageBoxWarning));
-            m_warningAction->setToolTip(warningText(lastTestResult).arg(validator->lastTestedPath()));
-        }
-    }
 }
 
 void Private::FileLineEdit::contextMenuEvent(QContextMenuEvent *event)
@@ -316,24 +239,58 @@ void Private::FileLineEdit::showCompletionPopup()
     m_completer->complete();
 }
 
-QString Private::FileLineEdit::warningText(FileSystemPathValidator::TestResult r)
+void Private::FileLineEdit::validateText()
+{
+    const auto *validator = qobject_cast<const FileSystemPathValidator *>(this->validator());
+    if (!validator)
+        return;
+
+    const FileSystemPathValidator::TestResult lastTestResult = validator->lastTestResult();
+    const QValidator::State lastState = validator->lastValidationState();
+
+    if (lastTestResult == FileSystemPathValidator::TestResult::OK)
+    {
+        delete m_warningAction;
+        m_warningAction = nullptr;
+    }
+    else
+    {
+        if (!m_warningAction)
+        {
+            m_warningAction = new QAction(this);
+            addAction(m_warningAction, QLineEdit::TrailingPosition);
+        }
+    }
+
+    if (m_warningAction)
+    {
+        if (lastState == QValidator::Invalid)
+            m_warningAction->setIcon(style()->standardIcon(QStyle::SP_MessageBoxCritical));
+        else if (lastState == QValidator::Intermediate)
+            m_warningAction->setIcon(style()->standardIcon(QStyle::SP_MessageBoxInformation));
+        m_warningAction->setToolTip(warningText(lastTestResult));
+    }
+}
+
+QString Private::FileLineEdit::warningText(const FileSystemPathValidator::TestResult result)
 {
     using TestResult = FileSystemPathValidator::TestResult;
-    switch (r)
+    switch (result)
     {
     case TestResult::DoesNotExist:
-        return tr("'%1' does not exist");
+        return tr("Path does not exist");
     case TestResult::NotADir:
-        return tr("'%1' does not point to a directory");
+        return tr("Path does not point to a directory");
     case TestResult::NotAFile:
-        return tr("'%1' does not point to a file");
+        return tr("Path does not point to a file");
     case TestResult::CantRead:
-        return tr("Does not have read permission in '%1'");
+        return tr("Don't have read permission to path");
     case TestResult::CantWrite:
-        return tr("Does not have write permission in '%1'");
+        return tr("Don't have write permission to path");
     default:
-        return {};
+        break;
     }
+    return {};
 }
 
 Private::FileComboEdit::FileComboEdit(QWidget *parent)

src/gui/fspathedit_p.h

@@ -50,60 +50,55 @@ namespace Private
         Q_DISABLE_COPY_MOVE(FileSystemPathValidator)
 
     public:
+        enum class TestResult
+        {
+            OK,
+            DoesNotExist,
+            NotADir,
+            NotAFile,
+            CantRead,
+            CantWrite
+        };
+
         FileSystemPathValidator(QObject *parent = nullptr);
 
         bool strictMode() const;
-        void setStrictMode(bool v);
+        void setStrictMode(bool value);
 
         bool existingOnly() const;
-        void setExistingOnly(bool v);
+        void setExistingOnly(bool value);
 
         bool directoriesOnly() const;
-        void setDirectoriesOnly(bool v);
+        void setDirectoriesOnly(bool value);
 
         bool checkReadPermission() const;
-        void setCheckReadPermission(bool v);
+        void setCheckReadPermission(bool value);
 
         bool checkWritePermission() const;
-        void setCheckWritePermission(bool v);
-
-        QValidator::State validate(QString &input, int &pos) const override;
-
-        enum class TestResult
-        {
-            OK,
-            DoesNotExist,
-            NotADir,
-            NotAFile,
-            CantRead,
-            CantWrite
-        };
+        void setCheckWritePermission(bool value);
 
         TestResult lastTestResult() const;
         QValidator::State lastValidationState() const;
-        QString lastTestedPath() const;
 
-    private:
-        QValidator::State validate(const QList<QStringView> &pathComponents, bool strict,
-                                   int firstComponentToTest, int lastComponentToTest) const;
+        QValidator::State validate(QString &input, int &pos) const override;
 
-        TestResult testPath(const Path &path, bool pathIsComplete) const;
+    private:
+        TestResult testPath(const Path &path) const;
 
-        bool m_strictMode;
-        bool m_existingOnly;
-        bool m_directoriesOnly;
-        bool m_checkReadPermission;
-        bool m_checkWritePermission;
+        bool m_strictMode = false;
+        bool m_existingOnly = false;
+        bool m_directoriesOnly = false;
+        bool m_checkReadPermission = false;
+        bool m_checkWritePermission = false;
 
-        mutable TestResult m_lastTestResult;
-        mutable QValidator::State m_lastValidationState;
-        mutable QString m_lastTestedPath;
+        mutable TestResult m_lastTestResult = TestResult::DoesNotExist;
+        mutable QValidator::State m_lastValidationState = QValidator::Invalid;
     };
 
-    class FileEditorWithCompletion
+    class IFileEditorWithCompletion
     {
     public:
-        virtual ~FileEditorWithCompletion() = default;
+        virtual ~IFileEditorWithCompletion() = default;
         virtual void completeDirectoriesOnly(bool completeDirsOnly) = 0;
         virtual void setFilenameFilters(const QStringList &filters) = 0;
         virtual void setBrowseAction(QAction *action) = 0;
@@ -113,7 +108,7 @@ namespace Private
         virtual QWidget *widget() = 0;
     };
 
-    class FileLineEdit final : public QLineEdit, public FileEditorWithCompletion
+    class FileLineEdit final : public QLineEdit, public IFileEditorWithCompletion
     {
         Q_OBJECT
         Q_DISABLE_COPY_MOVE(FileLineEdit)
@@ -135,8 +130,10 @@ namespace Private
         void contextMenuEvent(QContextMenuEvent *event) override;
 
     private:
-        static QString warningText(FileSystemPathValidator::TestResult r);
         void showCompletionPopup();
+        void validateText();
+
+        static QString warningText(FileSystemPathValidator::TestResult result);
 
         QFileSystemModel *m_completerModel = nullptr;
         QCompleter *m_completer = nullptr;
@@ -145,7 +142,7 @@ namespace Private
         QFileIconProvider m_iconProvider;
     };
 
-    class FileComboEdit final : public QComboBox, public FileEditorWithCompletion
+    class FileComboEdit final : public QComboBox, public IFileEditorWithCompletion
     {
         Q_OBJECT
         Q_DISABLE_COPY_MOVE(FileComboEdit)