Skip to content

fix(editor): hardcode CaseSensitive in replace skip to fix highlight#447

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
pengfeixx:fix/replace-skip-case-sensitive-highlight
Apr 22, 2026
Merged

fix(editor): hardcode CaseSensitive in replace skip to fix highlight#447
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
pengfeixx:fix/replace-skip-case-sensitive-highlight

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

@pengfeixx pengfeixx commented Apr 22, 2026

Remove Qt::CaseSensitivity from replaceSkip signal to avoid QueuedConnection enum serialization failure. Hardcode CaseSensitive directly in handleReplaceSkip for correct case-sensitive highlight.

替换跳过信号不再传递caseFlag枚举参数,避免QueuedConnection
序列化丢失。在handleReplaceSkip中直接使用CaseSensitive,
确保跳过后仅高亮精确匹配的内容。

Log: 修复替换跳过后大小写不敏感的高亮问题
PMS: BUG-358129
Influence: 替换跳过后高亮仅匹配相同大小写的内容,不再误匹配不同大小写的文本。

Summary by Sourcery

Ensure replace-skip operations always use case-sensitive matching and highlighting by simplifying the replaceSkip signal/slot interface.

Bug Fixes:

  • Fix incorrect case-insensitive highlighting after skipping a replace by forcing case-sensitive matching in the skip handler.

Tests:

  • Update the replace-skip unit test to reflect the new signal/slot signature without the case-sensitivity parameter.

Remove Qt::CaseSensitivity from replaceSkip signal to avoid
QueuedConnection enum serialization failure. Hardcode CaseSensitive
directly in handleReplaceSkip for correct case-sensitive highlight.

替换跳过信号不再传递caseFlag枚举参数,避免QueuedConnection
序列化丢失。在handleReplaceSkip中直接使用CaseSensitive,
确保跳过后仅高亮精确匹配的内容。

Log: 修复替换跳过后大小写不敏感的高亮问题
PMS: BUG-358129
Influence: 替换跳过后高亮仅匹配相同大小写的内容,不再误匹配不同大小写的文本。
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 22, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Hardens the replace-skip workflow by removing the CaseSensitivity enum from the replaceSkip signal/slot interface and instead always using Qt::CaseSensitive inside Window::handleReplaceSkip, preventing QueuedConnection enum serialization issues and ensuring case-sensitive highlighting after skipping replacements.

Sequence diagram for updated replaceSkip interaction

sequenceDiagram
    actor User
    participant ReplaceBar
    participant Window
    participant EditWrapper
    participant TextEditor

    User->>ReplaceBar: clickSkip()
    ReplaceBar->>Window: replaceSkip(file, keyword)
    Window->>Window: handleReplaceSkip(file, keyword)
    Window->>Window: handleUpdateSearchKeyword(m_replaceBar, file, keyword, Qt::CaseSensitive)
    Window->>EditWrapper: currentWrapper()
    EditWrapper-->>Window: wrapper
    alt m_keywordForSearchAll not equal m_keywordForSearch (caseInsensitive)
        Window->>TextEditor: clearFindMatchSelections()
    else equal
        Window->>TextEditor: highlightKeywordInView(m_keywordForSearchAll, Qt::CaseSensitive)
    end
Loading

Updated class diagram for ReplaceBar and Window replaceSkip handling

classDiagram
    class ReplaceBar {
        +void activeInput(QString text, QString file, int row, int column, int length)
        +void handleSkip()
        +void replaceClose()
        <<signal>> void pressEsc()
        <<signal>> void replaceNext(QString file, QString replaceText, QString withText)
        <<signal>> void replaceSkip(QString file, QString keyword)
        <<signal>> void replaceRest(QString replaceText, QString withText)
        <<signal>> void replaceAll(QString replaceText, QString withText)
        <<signal>> void beforeReplace(QString _)
    }

    class Window {
        +void handleReplaceAll(const QString replaceText, const QString withText)
        +void handleReplaceNext(const QString file, const QString replaceText, const QString withText)
        +void handleReplaceRest(const QString replaceText, const QString withText)
        +void handleReplaceSkip(QString file, QString keyword)
        +void handleRemoveSearchKeyword()
        +void handleUpdateSearchKeyword(QWidget widget, const QString file, const QString keyword, Qt::CaseSensitivity caseSensitivity)
        QString m_keywordForSearch
        QString m_keywordForSearchAll
        EditWrapper* currentWrapper()
    }

    class EditWrapper {
        +TextEditor* textEditor()
    }

    class TextEditor {
        +void clearFindMatchSelections()
        +void highlightKeywordInView(const QString keyword, Qt::CaseSensitivity caseSensitivity)
    }

    ReplaceBar ..> Window : signal replaceSkip
    Window ..> EditWrapper : uses
    EditWrapper ..> TextEditor : uses
    Window ..> TextEditor : calls highlighting methods
Loading

File-Level Changes

Change Details Files
Refactor replaceSkip signal/slot to no longer transmit CaseSensitivity and enforce case-sensitive behavior within the handler.
  • Remove Qt::CaseSensitivity parameter from ReplaceBar::replaceSkip signal and its declaration in replacebar.h.
  • Update ReplaceBar::handleSkip to emit replaceSkip without the CaseSensitivity argument.
  • Adjust Window::handleReplaceSkip slot signature to drop the CaseSensitivity parameter and update debug logging accordingly.
  • In Window::handleReplaceSkip, always call handleUpdateSearchKeyword with Qt::CaseSensitive instead of a parameter-provided value.
  • In Window::handleReplaceSkip, always call highlightKeywordInView with Qt::CaseSensitive to ensure only exact case matches are highlighted after skip.
  • Update unit test UT_Window_handleReplaceSkip to call the new handleReplaceSkip signature without CaseSensitivity.
src/controls/replacebar.cpp
src/controls/replacebar.h
src/widgets/window.cpp
src/widgets/window.h
tests/src/widgets/ut_window.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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码修改主要是移除了 replaceSkip 相关函数中的 Qt::CaseSensitivity caseFlag 参数,并在实现中硬编码为 Qt::CaseSensitive。以下是对这段代码的详细审查意见:

1. 语法逻辑

  • 语法正确性:代码修改在语法层面没有问题,Qt 的信号槽机制支持参数数量的变化,只要连接正确即可。
  • 逻辑一致性:修改将原本可配置的大小写敏感参数移除,统一改为大小写敏感(Qt::CaseSensitive)。这会导致所有替换操作都强制区分大小写,可能不符合用户预期(特别是当用户之前使用了不区分大小写的搜索时)。

2. 代码质量

  • 硬编码问题:将 Qt::CaseSensitive 硬编码在 Window::handleReplaceSkip 中会降低代码的灵活性。如果未来需要支持大小写不敏感的替换,需要重新修改代码。
  • 参数传递不一致replaceSkip 信号移除了 caseFlag 参数,但其他类似函数(如 replaceNext)仍然保留了相关参数,导致接口设计不一致。

3. 代码性能

  • 性能影响:移除参数对性能几乎没有影响,因为 Qt::CaseSensitive 是一个枚举值,传递开销极小。
  • 潜在性能问题:如果 handleUpdateSearchKeywordhighlightKeywordInView 内部对大小写敏感的处理有性能差异,硬编码可能导致无法优化某些场景。

4. 代码安全

  • 潜在功能缺失:移除 caseFlag 参数可能导致用户无法控制替换操作的大小写敏感性,从而影响功能的完整性。例如,用户可能期望在不区分大小写的搜索后执行替换操作,但修改后无法实现。
  • 测试覆盖:测试代码 ut_window.cpp 中的 handleReplaceSkip 测试用例也移除了 caseFlag 参数,但未验证大小写敏感的逻辑是否正确。

改进建议

  1. 恢复 caseFlag 参数

    • 建议保留 caseFlag 参数,允许调用者控制大小写敏感性。例如:
      void replaceSkip(QString file, QString keyword, Qt::CaseSensitivity caseFlag);
    • 如果确实需要默认行为,可以提供重载函数:
      void replaceSkip(QString file, QString keyword) {
          replaceSkip(file, keyword, Qt::CaseSensitive);
      }
  2. 统一接口设计

    • 检查其他替换相关函数(如 replaceNextreplaceRest)的参数设计,确保接口一致性。例如:
      void replaceNext(QString file, QString replaceText, QString withText, Qt::CaseSensitivity caseFlag);
  3. 添加文档注释

    • 在函数声明处添加注释,说明 caseFlag 的作用和默认行为:
      /**
       * @brief Skip the current replacement and move to the next match.
       * @param file The file being edited.
       * @param keyword The keyword to replace.
       * @param caseFlag Case sensitivity flag (default: Qt::CaseSensitive).
       */
      void replaceSkip(QString file, QString keyword, Qt::CaseSensitivity caseFlag = Qt::CaseSensitive);
  4. 增强测试用例

    • 在测试中验证大小写敏感和不敏感的场景:
      TEST(UT_Window_handleReplaceSkip, UT_Window_handleReplaceSkip_CaseSensitive) {
          window->handleReplaceSkip("aa", "Aa", Qt::CaseSensitive);
          // 验证是否区分大小写
      }
      TEST(UT_Window_handleReplaceSkip, UT_Window_handleReplaceSkip_CaseInsensitive) {
          window->handleReplaceSkip("aa", "Aa", Qt::CaseInsensitive);
          // 验证是否不区分大小写
      }
  5. 考虑用户配置

    • 如果大小写敏感性是全局配置(如从设置中读取),可以改为从配置中获取:
      Qt::CaseSensitivity caseFlag = m_settings->value("search/case_sensitive", true).toBool() 
                                    ? Qt::CaseSensitive : Qt::CaseInsensitive;

总结

这段代码修改虽然语法正确,但牺牲了功能的灵活性和一致性。建议恢复 caseFlag 参数,并通过默认值或配置管理来简化调用。同时,增强测试用例以确保大小写敏感逻辑的正确性。

Copy link
Copy Markdown

@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:

  • Consider adding a brief code comment near replaceSkip / handleReplaceSkip explaining why the case sensitivity is now hardcoded (QueuedConnection enum serialization issue), so future refactors don’t inadvertently reintroduce the enum parameter.
  • Since Qt::CaseSensitive is now the fixed behavior, you could centralize this choice (e.g., a local constexpr or helper) instead of repeating the literal in multiple calls, which will make future behavior changes easier.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider adding a brief code comment near `replaceSkip` / `handleReplaceSkip` explaining why the case sensitivity is now hardcoded (QueuedConnection enum serialization issue), so future refactors don’t inadvertently reintroduce the enum parameter.
- Since `Qt::CaseSensitive` is now the fixed behavior, you could centralize this choice (e.g., a local constexpr or helper) instead of repeating the literal in multiple calls, which will make future behavior changes easier.

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.

@github-actions
Copy link
Copy Markdown

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "src/widgets/window.cpp": [
        {
            "line": "    QString key = \"base/enable\";",
            "line_number": 389,
            "rule": "S106",
            "reason": "Var naming | 64f28539d9"
        }
    ]
}

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, pengfeixx

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

@pengfeixx
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit 08fb44e into linuxdeepin:master Apr 22, 2026
22 checks passed
@pengfeixx pengfeixx deleted the fix/replace-skip-case-sensitive-highlight branch April 22, 2026 06:22
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