Skip to content

fix: Skip case insensitive#448

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
JWWTSL:master
Apr 23, 2026
Merged

fix: Skip case insensitive#448
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
JWWTSL:master

Conversation

@JWWTSL
Copy link
Copy Markdown
Contributor

@JWWTSL JWWTSL commented Apr 22, 2026

log: m_searchCaseFlag was not persisted. After a replace bar operation, any cursor movement or scrolling would re-highlight using the default CaseInsensitive, overwriting the correct case-sensitive setting. Added m_searchCaseFlag to store the current search case flag, set replace bar related operations to CaseSensitive uniformly, and changed all highlightKeywordInView calls to read this flag.

pms: bug-358129

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.

Sorry @JWWTSL, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@github-actions
Copy link
Copy Markdown

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

@github-actions
Copy link
Copy Markdown

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

@github-actions
Copy link
Copy Markdown

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

log: m_searchCaseFlag was not persisted. After a replace bar operation, any cursor movement or scrolling would re-highlight using the default CaseInsensitive, overwriting the correct case-sensitive setting. Added m_searchCaseFlag to store the current search case flag, set replace bar related operations to CaseSensitive uniformly, and changed all highlightKeywordInView calls to read this flag.

pms: bug-358129
@github-actions
Copy link
Copy Markdown

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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

Git Diff 代码审查报告

总体评价

这段代码主要实现了文本编辑器中搜索高亮功能的大小写敏感性支持。代码修改了多个函数,增加了对大小写敏感标志的传递和使用。整体修改逻辑清晰,但存在一些可以改进的地方。

详细审查意见

1. 语法与逻辑问题

  1. 拼写错误

    • findBarIsVisiable()replaceBarIsVisiable() 方法名拼写错误,应为 findBarIsVisible()replaceBarIsVisible()
    • 建议修复这些拼写错误,以符合标准命名规范。
  2. 重复代码

    • nextLine(), prevLine(), scrollUp(), scrollDown() 四个函数中存在大量重复代码。
    • 建议提取公共逻辑到一个单独的函数中,减少代码重复。
  3. 默认值不一致

    • popupFindBar() 中设置 m_searchCaseFlag = Qt::CaseInsensitive,而 popupReplaceBar() 中设置 m_searchCaseFlag = Qt::CaseSensitive
    • 建议在类初始化时统一设置默认值,避免不一致。

2. 代码质量问题

  1. 空行过多

    • window.cpp 文件中 resizeEvent() 后有多余的空行:
      void Window::resizeEvent(QResizeEvent *e)
      {
          DMainWindow::resizeEvent(e);
      }
      
      
      void Window::closeEvent(QCloseEvent *e)
    • 建议删除多余空行,保持代码整洁。
  2. 函数参数传递

    • highlightKeywordInView() 函数现在接受两个参数,但调用处需要确保第二个参数始终正确传递。
    • 建议考虑使用默认参数值,如 highlightKeywordInView(const QString &keyword, Qt::CaseSensitivity caseFlag = Qt::CaseInsensitive)
  3. 日志输出

    • 代码中有多处 qDebug() 输出,建议考虑使用条件编译或日志级别控制,避免在发布版本中输出过多调试信息。

3. 性能问题

  1. 频繁调用 window() 方法

    • 在多个函数中,m_wrapper->window() 被多次调用。
    • 修改中已经通过 Window *win = m_wrapper->window(); 缓存了窗口指针,这是好的实践,建议在所有类似场景中应用。
  2. 字符串比较

    • QString::compare() 被用于比较搜索关键词,这在频繁调用时可能会有性能影响。
    • 考虑使用 QString::operator==QString::compare 的无返回值版本,如果不需要比较结果的具体值。

4. 安全性问题

  1. 空指针检查

    • 虽然代码中有 if (m_wrapper != nullptr) 检查,但对 m_wrapper->window() 的返回值没有进行检查。
    • 建议添加对 win 指针的空指针检查:
      Window *win = m_wrapper->window();
      if (!win) {
          return;
      }
  2. 成员变量初始化

    • m_searchCaseFlag 在类中初始化为 Qt::CaseInsensitive,但在多个地方被重新赋值。
    • 建议确保所有代码路径都正确初始化此变量,避免使用未初始化的值。

改进建议代码

1. 提取重复代码

// 在 TextEdit 类中添加新方法
void TextEdit::updateHighlighterIfNeeded()
{
    if (m_wrapper == nullptr) {
        return;
    }
    
    qDebug() << "Updating highlighter";
    m_wrapper->OnUpdateHighlighter();
    
    Window *win = m_wrapper->window();
    if (!win) {
        return;
    }
    
    const QString keyword = win->getKeywordForSearchAll();
    if ((win->findBarIsVisible() || win->replaceBarIsVisible()) &&
        (QString::compare(keyword, win->getKeywordForSearch(), Qt::CaseInsensitive) == 0)) {
        qDebug() << "Highlighting keyword in view";
        highlightKeywordInView(keyword, win->getSearchCaseFlag());
    }
    
    qDebug() << "Marking all keyword in view";
    markAllKeywordInView();
}

// 然后修改原函数
void TextEdit::nextLine()
{
    // ... 原有代码 ...
    updateHighlighterIfNeeded();
    // ... 原有代码 ...
}

2. 修复拼写错误并添加空指针检查

// 在 Window 类中修改方法名
bool Window::findBarIsVisible()
{
    // ...
}

bool Window::replaceBarIsVisible()
{
    // ...
}

// 在 TextEdit 类中使用
void TextEdit::nextLine()
{
    // ...
    if (m_wrapper != nullptr) {
        Window *win = m_wrapper->window();
        if (!win) {
            return;
        }
        // ...
    }
    // ...
}

3. 使用默认参数值

// 在 TextEdit 类中修改方法声明
void highlightKeywordInView(const QString &keyword, Qt::CaseSensitivity caseFlag = Qt::CaseInsensitive);

4. 统一初始化大小写敏感标志

// 在 Window 类构造函数中
Window::Window(DMainWindow *parent) : DMainWindow(parent)
{
    // ...
    m_searchCaseFlag = Qt::CaseInsensitive; // 统一初始化默认值
    // ...
}

总结

这段代码修改主要实现了搜索高亮功能的大小写敏感性支持,整体逻辑正确,但存在一些可以改进的地方:

  1. 修复拼写错误
  2. 提取重复代码以减少冗余
  3. 添加更全面的空指针检查
  4. 统一初始化成员变量
  5. 考虑使用默认参数值简化函数调用
  6. 优化日志输出

通过这些改进,可以提高代码的可读性、可维护性和健壮性。

@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

@JWWTSL
Copy link
Copy Markdown
Contributor Author

JWWTSL commented Apr 23, 2026

/forcemerge

@deepin-bot deepin-bot Bot merged commit be1805a into linuxdeepin:master Apr 23, 2026
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