Skip to content

Revert "fix: Skip case insensitive"#449

Merged
lzwind merged 1 commit intolinuxdeepin:masterfrom
JWWTSL:revert-448-master
Apr 23, 2026
Merged

Revert "fix: Skip case insensitive"#449
lzwind merged 1 commit intolinuxdeepin:masterfrom
JWWTSL:revert-448-master

Conversation

@JWWTSL
Copy link
Copy Markdown
Contributor

@JWWTSL JWWTSL commented Apr 23, 2026

Reverts #448

Summary by Sourcery

Revert the previous change that introduced explicit case-sensitivity handling for search highlighting and restore the prior default behavior.

Bug Fixes:

  • Remove explicit tracking of search case sensitivity in the main window and rely on the editor’s default search behavior to avoid regressions introduced by the prior fix.

Enhancements:

  • Simplify search/highlight integration between the window, edit wrapper, and text editor by dropping the search case flag parameter and associated state.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

CLA Assistant Lite bot:

如果你是以企业贡献者的身份进行提交,请联系我们签署企业贡献者许可协议
If you submit as corporate contributor, please contact us to sign our Corporate Contributor License Agreement

感谢您的提交,我们非常感谢。 像许多开源项目一样,在接受您的贡献之前,我们要求您签署我们的个人贡献者许可协议。 您只需发布与以下格式相同的评论即可签署个人贡献者许可协议
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Individual Contributor License Agreement before we can accept your contribution. You can sign the Individual Contributor License Agreement by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA.

You can retrigger this bot by commenting recheck in this Pull Request

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 23, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Reverts the previous case-sensitivity-aware search highlighting behavior, simplifying keyword highlighting to always use the stored "search all" keyword without tracking or propagating a case-sensitivity flag, and inlining some window access instead of using temporaries.

Sequence diagram for reverted find bar search highlighting flow

sequenceDiagram
    actor User
    participant Window
    participant EditWrapper
    participant TextEdit

    User->>Window: popupFindBar()
    activate Window
    Window->>EditWrapper: currentWrapper()
    activate EditWrapper
    EditWrapper-->>Window: wrapper
    deactivate EditWrapper

    Window->>TextEdit: textEditor()
    activate TextEdit
    TextEdit-->>Window: editor
    deactivate TextEdit

    Window->>Window: compute text, tabPath, row, column, scrollOffset
    Window->>FindBar: activeInput(text, tabPath, row, column, scrollOffset)

    Note over Window,TextEdit: Reverted behavior: no case flag is set on Window

    Window->>TextEdit: highlightKeywordInView(text)
    activate TextEdit
    TextEdit-->>Window: highlighted
    deactivate TextEdit

    Window->>Window: m_keywordForSearchAll = text
    Window->>Window: m_keywordForSearch = text
    deactivate Window
Loading

Sequence diagram for reverted scroll-triggered highlighting via EditWrapper

sequenceDiagram
    participant Window
    participant EditWrapper
    participant TextEdit

    EditWrapper->>EditWrapper: connect(verticalScrollBar valueChanged)

    TextEdit->>EditWrapper: verticalScrollBar valueChanged
    activate EditWrapper
    EditWrapper->>Window: getKeywordForSearchAll()
    Window-->>EditWrapper: m_keywordForSearchAll
    EditWrapper->>Window: getKeywordForSearch()
    Window-->>EditWrapper: m_keywordForSearch

    EditWrapper->>EditWrapper: compare keywords (Qt::CaseInsensitive)
    alt bars visible and keywords equal
        EditWrapper->>Window: findBarIsVisiable(), replaceBarIsVisiable()
        Window-->>EditWrapper: visibility flags
        EditWrapper->>TextEdit: highlightKeywordInView(m_keywordForSearchAll)
        activate TextEdit
        TextEdit-->>EditWrapper: highlighted
        deactivate TextEdit
    end

    EditWrapper->>TextEdit: markAllKeywordInView()
    TextEdit-->>EditWrapper: all_marked
    deactivate EditWrapper
Loading

Class diagram for reverted search highlighting state management

classDiagram
    class Window {
        +Window(DMainWindow* parent)
        +void popupFindBar()
        +void popupReplaceBar()
        +void handleFindKeyword(const QString& keyword, bool state)
        +void handleReplaceNext(const QString& file, const QString& replaceText, const QString& withText)
        +void handleUpdateSearchKeyword(QWidget* widget, const QString& file, const QString& keyword, Qt::CaseSensitivity caseFlag)
        +bool findBarIsVisiable()
        +bool replaceBarIsVisiable()
        +QString getKeywordForSearchAll()
        +QString getKeywordForSearch()
        +void resizeEvent(QResizeEvent* e)
        +void closeEvent(QCloseEvent* e)
        +void setPrintEnabled(bool enabled)
        +QStackedWidget* getStackedWgt()
        -- search state --
        QString m_keywordForSearch
        QString m_keywordForSearchAll
    }

    class EditWrapper {
        +EditWrapper(Window* window, QWidget* parent)
        +Window* window()
        +TextEdit* textEditor()
        -Window* m_pWindow
        -TextEdit* m_pTextEdit
    }

    class TextEdit {
        +void nextLine()
        +void prevLine()
        +void scrollUp()
        +void scrollDown()
        +void highlightKeywordInView(const QString& keyword)
        +void markAllKeywordInView()
        +void clearFindMatchSelections()
        +bool highlightKeyword(const QString& keyword, int position, Qt::CaseSensitivity caseFlag)
        +int getScrollOffset()
        +int getPosition()
    }

    Window "1" o-- "many" EditWrapper : owns
    EditWrapper "1" o-- "1" TextEdit : wraps

    %% Key reverted change:
    %% - Window no longer stores m_searchCaseFlag or getSearchCaseFlag
    %% - highlightKeywordInView is now called without a case flag everywhere
Loading

File-Level Changes

Change Details Files
Remove propagation and storage of a search case-sensitivity flag and revert highlight calls to use only the stored keyword.
  • Change TextEdit navigation and scroll handlers to call highlightKeywordInView with only the "search all" keyword instead of passing a case flag from Window.
  • Update EditWrapper scroll handling to drop use of Window::getSearchCaseFlag and only pass the keyword to highlightKeywordInView.
  • Remove m_searchCaseFlag member and getSearchCaseFlag() accessor from Window, and stop updating this flag in replace/find handlers.
src/editor/dtextedit.cpp
src/editor/editwrapper.cpp
src/widgets/window.cpp
src/widgets/window.h
Revert case-sensitivity defaults for find/replace bars and simplify related wiring.
  • Change ReplaceBar::updateSearchKeyword connection to call handleUpdateSearchKeyword without forcing Qt::CaseSensitive.
  • Stop forcing case-insensitive search when popping up the find bar and remove the "findbar uses case-insensitive by default" behavior.
  • Stop forcing case-sensitive behavior when popping up the replace bar and when handling replace-next operations.
src/widgets/window.cpp
Minor cleanups related to the revert.
  • Inline m_wrapper->window() usage instead of storing a local Window* in TextEdit methods.
  • Remove now-unused QTimer include from window.cpp and a couple of stray blank lines in resize/close event region.
src/editor/dtextedit.cpp
src/widgets/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

代码审查意见

这份 diff 主要对代码进行了重构,移除了 m_searchCaseFlag 成员变量及其相关的使用,简化了 highlightKeywordInView 函数的调用。以下是关于语法逻辑、代码质量、性能和安全的详细审查意见:

1. 语法逻辑

  • 重大逻辑变更风险
    • 代码移除了 Window 类中的成员变量 m_searchCaseFlag,并且在调用 highlightKeywordInView 时不再传递该参数。
    • src/widgets/window.cpp 中,原本的逻辑是:
      • popupFindBar: 设置 m_searchCaseFlag = Qt::CaseInsensitive
      • popupReplaceBar: 设置 m_searchCaseFlag = Qt::CaseSensitive
      • handleUpdateSearchKeyword: 根据传入的 caseFlag 更新 m_searchCaseFlag
    • 改进意见:当前的修改直接删除了大小写敏感的控制逻辑。如果 highlightKeywordInView 函数内部没有默认的大小写处理机制(例如默认总是不区分大小写,或者总是区分大小写),这将导致功能回退行为不一致
    • 建议:必须检查 TextEdit::highlightKeywordInView 的实现。如果该函数现在只有一个参数,请确认它是如何决定大小写敏感性的。
      • 如果是固定行为(例如总是不区分大小写),那么这个改动实际上是改变了功能,需要确认这是产品需求。
      • 如果需要保留原有功能,highlightKeywordInView 需要能够获取当前的搜索设置(区分/不区分),或者需要重新引入大小写敏感性的控制逻辑。如果是从 UI 控件(如 FindBarReplaceBar)获取状态,应确保在调用时能实时获取到。

2. 代码质量

  • 代码重复

    • src/editor/dtextedit.cppnextLine, prevLine, scrollUp, scrollDown 四个函数中,存在大量重复的逻辑判断代码:
      if ((m_wrapper->window()->findBarIsVisiable() || m_wrapper->window()->replaceBarIsVisiable()) &&
              (QString::compare(m_wrapper->window()->getKeywordForSearchAll(), m_wrapper->window()->getKeywordForSearch(), Qt::CaseInsensitive) == 0))
    • 改进意见:这段代码在每个函数中都重复出现,且涉及多次链式调用 m_wrapper->window()->...,可读性较差,且增加了维护成本。
    • 建议:将此判断逻辑提取为一个 TextEdit 类的私有辅助函数,例如 shouldHighlightKeywords()
      bool TextEdit::shouldHighlightKeywords() const {
          if (!m_wrapper) return false;
          auto win = m_wrapper->window();
          return (win->findBarIsVisiable() || win->replaceBarIsVisiable()) &&
                 (QString::compare(win->getKeywordForSearchAll(), win->getKeywordForSearch(), Qt::CaseInsensitive) == 0);
      }
      这样原来的调用处可以简化为 if (shouldHighlightKeywords()) { ... }
  • 变量作用域与链式调用

    • 修改前的代码使用了 Window *win = m_wrapper->window(); 局部变量,这减少了重复调用 m_wrapper->window() 的开销,且代码更清晰。
    • 修改后的代码直接使用 m_wrapper->window()->... 链式调用。
    • 改进意见:虽然删除局部变量减少了代码行数,但在同一作用域内多次调用 m_wrapper->window()(如 if 条件判断中调用了 3 次)是不推荐的。
    • 建议:保留局部变量 Window *win = m_wrapper->window();,既提高可读性,也避免了潜在的多次函数调用开销(尽管编译器可能会优化)。

3. 代码性能

  • 字符串比较
    • 代码中频繁使用 QString::compare(..., Qt::CaseInsensitive) == 0
    • 改进意见:如果这对性能有影响(例如在滚动事件 scrollUp/Down 中高频触发),可以考虑优化字符串比较的时机,或者确保 getKeywordForSearchAllgetKeywordForSearch 返回的是引用(const QString&),避免不必要的拷贝构造。
  • 频繁的 UI 状态查询
    • 在滚动和光标移动事件中频繁调用 findBarIsVisiable()replaceBarIsVisiable()
    • 建议:虽然这些通常是简单的 getter,但在高频触发的函数中,应尽量保持逻辑轻量。

4. 代码安全

  • 空指针检查
    • 代码中已经有 if (m_wrapper != nullptr) 的检查,这很好。
    • 建议:在提取出的辅助函数或重构后的代码中,确保始终检查 m_wrapperm_wrapper->window() 的有效性,防止潜在的空指针解引用。
  • 头文件清理
    • src/widgets/window.cpp 中移除了 #include <QTimer>
    • 改进意见:这是一个好的做法(减少依赖),但请确认 QTimer::singleShot 在该文件中是否还有其他地方使用。diff 中显示 popupReplaceBar 函数里还有 QTimer::singleShot(10, this, [ = ] { m_replaceBar->focus(); });
    • 严重问题:如果在 window.cpp 中仍有 QTimer 的使用却删除了头文件,会导致编译错误。请确认 popupReplaceBar 中的那段代码是否也被移除了,或者 QTimer 是通过其他头文件(如 Qt 框架的通用头文件)间接引入的。

总结建议

  1. 首要任务:确认 highlightKeywordInView 在移除 caseFlag 参数后的行为是否符合预期(即是否正确处理了大小写敏感逻辑)。
  2. 修复编译风险:检查 window.cpp 中是否还有 QTimer 的使用,如果有,需恢复头文件或移除相关代码。
  3. 重构优化:建议恢复 Window *win 局部变量以提高可读性,并将重复的条件判断提取为私有成员函数,消除 dtextedit.cpp 中的代码重复。

@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"
        }
    ]
}

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:

  • In the lambda connected to ReplaceBar::updateSearchKeyword, handleUpdateSearchKeyword is now called without the Qt::CaseSensitivity argument, so either the slot signature should be updated (e.g., default parameter/overload) or the lambda should still pass a case flag to keep the call consistent and compilable.
  • The repeated m_wrapper->window() calls inside nextLine, prevLine, scrollUp, and scrollDown were previously factored into a local Window *win; consider reintroducing a local variable to avoid repeated calls and improve readability.
  • With m_searchCaseFlag removed and highlightKeywordInView no longer taking a case flag, ensure that the different default behaviors for find vs. replace (case-insensitive vs. case-sensitive) are still expressible in the design, or consider introducing an explicit way to distinguish these modes if needed.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the lambda connected to `ReplaceBar::updateSearchKeyword`, `handleUpdateSearchKeyword` is now called without the `Qt::CaseSensitivity` argument, so either the slot signature should be updated (e.g., default parameter/overload) or the lambda should still pass a case flag to keep the call consistent and compilable.
- The repeated `m_wrapper->window()` calls inside `nextLine`, `prevLine`, `scrollUp`, and `scrollDown` were previously factored into a local `Window *win`; consider reintroducing a local variable to avoid repeated calls and improve readability.
- With `m_searchCaseFlag` removed and `highlightKeywordInView` no longer taking a case flag, ensure that the different default behaviors for find vs. replace (case-insensitive vs. case-sensitive) are still expressible in the design, or consider introducing an explicit way to distinguish these modes if needed.

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
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@lzwind lzwind merged commit 79658d3 into linuxdeepin:master Apr 23, 2026
19 of 22 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