Fix path validator

And revise tooltip message.

Closes #11152.

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,91 +91,36 @@ 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
-{
-    Q_ASSERT(firstComponentToTest >= 0);
-    Q_ASSERT(lastComponentToTest >= firstComponentToTest);
-    Q_ASSERT(lastComponentToTest < pathComponents.size());
-
-    m_lastTestResult = TestResult::DoesNotExist;
-    if (pathComponents.empty())
-        return strict ? QValidator::Invalid : QValidator::Intermediate;
-
-    for (int i = firstComponentToTest; i <= lastComponentToTest; ++i)
-    {
-        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;
-        }
-    }
-
-    return QValidator::Acceptable;
 }
 
 Private::FileSystemPathValidator::TestResult
-Private::FileSystemPathValidator::testPath(const Path &path, bool pathIsComplete) const
+Private::FileSystemPathValidator::testPath(const Path &path) const
 {
-    QFileInfo fi {path.data()};
+    // `QFileInfo` will cache the query results and avoid exessive querying to filesystem
-    if (m_existingOnly && !fi.exists())
+    const QFileInfo info {path.data()};
+
+    if (existingOnly() && !info.exists())
         return TestResult::DoesNotExist;
 
-    if ((!pathIsComplete || m_directoriesOnly) && !fi.isDir())
+    if (directoriesOnly())
-        return TestResult::NotADir;
-
-    if (pathIsComplete)
     {
-        if (!m_directoriesOnly && fi.isDir())
+        if (!info.isDir())
-            return TestResult::NotAFile;
+            return TestResult::NotADir;
-
-        if (m_checkWritePermission && (fi.exists() && !fi.isWritable()))
-            return TestResult::CantWrite;
-        if (m_checkReadPermission && !fi.isReadable())
-            return TestResult::CantRead;
     }
+    else
+    {
+        if (!info.isFile())
+            return TestResult::NotAFile;
+    }
+
+    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)
@@ -223,10 +170,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;
+    const QDir::Filters filters = QDir::NoDotAndDotDot
-    filters |= QDir::NoDotAndDotDot;
+        | (completeDirsOnly ? QDir::Dirs : QDir::AllEntries);
     m_completerModel->setFilter(filters);
 }
 
@@ -263,17 +210,18 @@ 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());
         showCompletionPopup();
     }
 
-    auto *validator = qobject_cast<const FileSystemPathValidator *>(this->validator());
+    const auto *validator = qobject_cast<const FileSystemPathValidator *>(this->validator());
     if (validator)
     {
-        FileSystemPathValidator::TestResult lastTestResult = validator->lastTestResult();
+        const FileSystemPathValidator::TestResult lastTestResult = validator->lastTestResult();
-        QValidator::State lastState = validator->lastValidationState();
+        const QValidator::State lastState = validator->lastValidationState();
         if (lastTestResult == FileSystemPathValidator::TestResult::OK)
         {
             delete m_warningAction;
@@ -294,7 +242,7 @@ void Private::FileLineEdit::keyPressEvent(QKeyEvent *e)
                 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()));
+            m_warningAction->setToolTip(warningText(lastTestResult));
         }
     }
 }
@@ -319,24 +267,25 @@ void Private::FileLineEdit::showCompletionPopup()
     m_completer->complete();
 }
 
-QString Private::FileLineEdit::warningText(FileSystemPathValidator::TestResult r)
+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,25 +50,6 @@ namespace Private
         Q_DISABLE_COPY_MOVE(FileSystemPathValidator)
 
     public:
-        FileSystemPathValidator(QObject *parent = nullptr);
-
-        bool strictMode() const;
-        void setStrictMode(bool v);
-
-        bool existingOnly() const;
-        void setExistingOnly(bool v);
-
-        bool directoriesOnly() const;
-        void setDirectoriesOnly(bool v);
-
-        bool checkReadPermission() const;
-        void setCheckReadPermission(bool v);
-
-        bool checkWritePermission() const;
-        void setCheckWritePermission(bool v);
-
-        QValidator::State validate(QString &input, int &pos) const override;
-
         enum class TestResult
         {
             OK,
@@ -79,25 +60,39 @@ namespace Private
             CantWrite
         };
 
+        FileSystemPathValidator(QObject *parent = nullptr);
+
+        bool strictMode() const;
+        void setStrictMode(bool value);
+
+        bool existingOnly() const;
+        void setExistingOnly(bool value);
+
+        bool directoriesOnly() const;
+        void setDirectoriesOnly(bool value);
+
+        bool checkReadPermission() const;
+        void setCheckReadPermission(bool value);
+
+        bool checkWritePermission() const;
+        void setCheckWritePermission(bool value);
+
         TestResult lastTestResult() const;
         QValidator::State lastValidationState() const;
-        QString lastTestedPath() const;
+
+        QValidator::State validate(QString &input, int &pos) const override;
 
     private:
-        QValidator::State validate(const QList<QStringView> &pathComponents, bool strict,
+        TestResult testPath(const Path &path) const;
-                                   int firstComponentToTest, int lastComponentToTest) const;
 
-        TestResult testPath(const Path &path, bool pathIsComplete) const;
+        bool m_strictMode = false;
+        bool m_existingOnly = false;
+        bool m_directoriesOnly = false;
+        bool m_checkReadPermission = false;
+        bool m_checkWritePermission = false;
 
-        bool m_strictMode;
+        mutable TestResult m_lastTestResult = TestResult::DoesNotExist;
-        bool m_existingOnly;
+        mutable QValidator::State m_lastValidationState = QValidator::Invalid;
-        bool m_directoriesOnly;
-        bool m_checkReadPermission;
-        bool m_checkWritePermission;
-
-        mutable TestResult m_lastTestResult;
-        mutable QValidator::State m_lastValidationState;
-        mutable QString m_lastTestedPath;
     };
 
     class FileEditorWithCompletion
@@ -135,7 +130,7 @@ namespace Private
         void contextMenuEvent(QContextMenuEvent *event) override;
 
     private:
-        static QString warningText(FileSystemPathValidator::TestResult r);
+        static QString warningText(FileSystemPathValidator::TestResult result);
         void showCompletionPopup();
 
         QFileSystemModel *m_completerModel = nullptr;