Skip to content

Conversation

@LiHua000
Copy link
Contributor

@LiHua000 LiHua000 commented Dec 25, 2025

log: Validate input only on focus loss instead of on every keystroke.

Bug:https://pms.uniontech.com/bug-view-345447.html

Summary by Sourcery

Adjust rename dialog input handling to validate and clean illegal characters only after editing is finished, preventing cursor position issues during typing while keeping confirmation button state tied to non-empty input.

Bug Fixes:

  • Prevent the rename dialog cursor from jumping to the end of the text while typing by avoiding live text mutation on each keystroke.

Enhancements:

  • Defer illegal-character cleanup in the rename dialog to when the field loses focus, preserving the final text while maintaining validation.

log: Validate input only on focus loss instead of on every keystroke.

Bug:https://pms.uniontech.com/bug-view-345447.html
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个diff进行代码审查:

  1. 代码逻辑改进:

    • 将非法字符过滤从实时过滤改为失去焦点时处理,这是一个很好的改进。这样可以避免在用户输入过程中频繁修改输入内容,提升用户体验。
    • 保留了空输入时禁用确认按钮的逻辑,这是必要的。
  2. 代码质量建议:

    • 正则表达式字符串 name 应该定义为常量,避免重复定义。建议在类中定义为静态常量。
    • 可以将清理非法字符的逻辑抽取为单独的函数,提高代码复用性。
    • 建议添加注释说明正则表达式的作用,特别是对于特殊字符的处理。
  3. 代码性能优化:

    • 当前实现在每次失去焦点时都会创建新的QRegularExpression对象,建议将正则表达式对象也定义为常量复用。
  4. 代码安全性:

    • 建议添加对lineEdit指针的有效性检查,虽然在这里不太可能为空,但这是一个良好的编程习惯。

改进后的代码建议:

class RenameDialog {
private:
    static const QString INVALID_CHARS_REGEX;  // 定义为类静态常量
    static const QRegularExpression INVALID_CHARS_PATTERN;
};

const QString RenameDialog::INVALID_CHARS_REGEX = "(^\\s+|[/\\\\:*\"'?<>|\r\n\t])";
const QRegularExpression RenameDialog::INVALID_CHARS_PATTERN(INVALID_CHARS_REGEX);

// 在showDialog中:
connect(m_lineEdit, &DLineEdit::textChanged, [=](){
    QString text = m_lineEdit->text();
    if(m_nOkBtnIndex >= 0) {
        getButton(m_nOkBtnIndex)->setEnabled(!text.isEmpty());
    }
});

connect(m_lineEdit->lineEdit(), &QLineEdit::editingFinished, [=](){
    QLineEdit *lineEdit = m_lineEdit->lineEdit();
    if (!lineEdit) return;  // 安全性检查
    
    QString text = lineEdit->text();
    if(INVALID_CHARS_PATTERN.match(text).hasMatch() || text.startsWith(" ")) {
        lineEdit->setText(text.remove(INVALID_CHARS_PATTERN));
    }
});

这样的改进:

  1. 提高了代码的可维护性和可读性
  2. 避免了重复创建正则表达式对象
  3. 增加了安全性检查
  4. 简化了确认按钮的启用/禁用逻辑
  5. 将正则表达式定义为常量,便于统一管理和修改

@sourcery-ai
Copy link

sourcery-ai bot commented Dec 25, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts rename dialog validation so illegal characters are stripped only when editing finishes (focus loss), preventing the cursor from jumping to the end while still disabling the OK button on empty input.

Sequence diagram for rename dialog text validation on focus loss

sequenceDiagram
    actor User
    participant RenameDialog
    participant DLineEdit
    participant QLineEdit
    participant OkButton

    User->>RenameDialog: Open rename dialog
    RenameDialog->>DLineEdit: create m_lineEdit
    DLineEdit->>QLineEdit: create internal lineEdit
    RenameDialog->>DLineEdit: connect textChanged
    RenameDialog->>QLineEdit: connect editingFinished

    User->>QLineEdit: Type characters
    QLineEdit-->>DLineEdit: textChanged(text)
    DLineEdit-->>RenameDialog: textChanged(text)
    RenameDialog->>RenameDialog: Check if text is empty
    alt text is empty
        RenameDialog->>OkButton: setEnabled(false)
    else text is not empty
        RenameDialog->>OkButton: setEnabled(true)
    end

    User->>QLineEdit: Change focus (finish editing)
    QLineEdit-->>RenameDialog: editingFinished()
    RenameDialog->>RenameDialog: Validate text with regex
    alt text contains illegal characters or starts with space
        RenameDialog->>QLineEdit: setText(cleanedText)
    else text is valid
        Note over RenameDialog,QLineEdit: Text remains unchanged
    end
Loading

Class diagram for updated rename dialog validation logic

classDiagram
    class RenameDialog {
        - DLineEdit* m_lineEdit
        - int m_nOkBtnIndex
        + int showDialog(QString strReName, QString strAlias, QWidget* parent)
    }

    class DLineEdit {
        + QLineEdit* lineEdit()
        + signal textChanged(QString text)
    }

    class QLineEdit {
        + QString text()
        + void setText(QString text)
        + signal textChanged(QString text)
        + signal editingFinished()
    }

    class DButton {
        + void setEnabled(bool enabled)
    }

    RenameDialog *-- DLineEdit : owns
    DLineEdit *-- QLineEdit : wraps
    RenameDialog --> QLineEdit : uses for validation
    RenameDialog --> DButton : controls enabled state
Loading

File-Level Changes

Change Details Files
Decouple inline text sanitization from the textChanged handler and perform illegal character cleanup only when the line edit loses focus, while preserving button enable/disable logic during typing.
  • Remove immediate regex-based removal of illegal characters from the DLineEdit::textChanged lambda to avoid interfering with cursor position during typing.
  • Keep the existing logic that toggles the OK button enabled state based on whether the current text is empty or not.
  • Add a new QLineEdit::editingFinished connection that builds the illegal-character regular expression, checks for any matches or leading spaces, and removes them from the text when focus is lost before updating the line edit.
src/source/dialog/popupdialog.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The regular expression pattern for invalid characters is now duplicated between the textChanged and editingFinished handlers; consider extracting it into a shared constant or helper to keep the logic in sync.
  • The explicit text.startsWith(" ") check appears redundant given the (^\s+...) part of the regex already matches leading whitespace; you can likely simplify the condition to only rely on the regex match.
  • When updating the line edit text on editingFinished, consider whether you need to preserve the cursor position/selection, as calling setText resets it and may affect UX if focus returns to the field.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The regular expression pattern for invalid characters is now duplicated between the `textChanged` and `editingFinished` handlers; consider extracting it into a shared constant or helper to keep the logic in sync.
- The explicit `text.startsWith(" ")` check appears redundant given the `(^\s+...)` part of the regex already matches leading whitespace; you can likely simplify the condition to only rely on the regex match.
- When updating the line edit text on `editingFinished`, consider whether you need to preserve the cursor position/selection, as calling `setText` resets it and may affect UX if focus returns to the field.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, max-lvs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@LiHua000
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 1ced6a2 into linuxdeepin:release/eagle Dec 25, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants