Skip to content

fix: Optimize mark removal logic in TextEdit and enhance file path ha…#384

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master
Sep 2, 2025
Merged

fix: Optimize mark removal logic in TextEdit and enhance file path ha…#384
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master

Conversation

@dengzhongyuan365-dev

@dengzhongyuan365-dev dengzhongyuan365-dev commented Sep 2, 2025

Copy link
Copy Markdown
Member

…ndling in Window

  • Refactored the mark removal process in TextEdit::updateMark to use QSet and removeIf, improving code clarity and reducing index management issues.

log: Optimize mark removal logic and enhance file path handling

bug: https://pms.uniontech.com/bug-view-331945.html

Summary by Sourcery

Refactor TextEdit::updateMark to simplify mark removal and avoid index mismanagement

Bug Fixes:

  • Fix incorrect index adjustments when removing word mark selections

Enhancements:

  • Use QSet and removeIf to optimize mark removal logic
  • Add debug logging to report the number of removed and remaining marks

…ndling in Window

- Refactored the mark removal process in TextEdit::updateMark to use QSet and removeIf, improving code clarity and reducing index management issues.

log: Optimize mark removal logic and enhance file path handling

bug: https://pms.uniontech.com/bug-view-331945.html
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

我对这段代码进行审查,并提出以下改进意见:

  1. 代码逻辑问题:
  • 原代码中的注释表明存在一个已知的索引管理问题("-j"相关的问题),这可能导致错误的标记移除
  • 原代码使用简单的循环移除元素,这在移除多个元素时可能会导致索引错位
  1. 代码质量改进:
  • 引入了QSet来存储要移除的索引,提高了查找效率
  • 使用removeIf算法替代手动循环,使代码更简洁易读
  • 添加了调试输出,便于追踪标记移除操作
  1. 代码性能改进:
  • 原代码在循环中调用removeAt,每次操作都可能触发O(n)的移动操作
  • 新代码使用removeIf一次性处理所有移除操作,减少了不必要的元素移动
  • QSet的查找操作是O(1)时间复杂度,比原来的线性查找更高效
  1. 代码安全性改进:
  • 添加了isEmpty()检查,避免在空列表上执行不必要的操作
  • 使用lambda表达式确保索引正确递增,避免越界访问
  • 使用auto类型推导,提高代码的健壮性
  1. 潜在问题:
  • 调试输出(qDebug)可能会影响性能,在发布版本中应该考虑移除或使用条件编译
  • index变量在lambda中通过引用捕获,需要确保其生命周期正确

建议的进一步改进:

  1. 考虑将调试输出改为使用Qt的qCDebug宏,这样可以在发布版本中轻松禁用
  2. 可以考虑将移除操作封装为一个单独的函数,提高代码复用性
  3. 添加适当的注释,说明removeSet和index的用途,提高代码可读性

改进后的代码已经解决了原代码中的索引管理问题,并提高了代码的性能和可维护性。不过,建议在实际应用中根据具体需求进行测试,确保修改后的代码符合预期行为。

@sourcery-ai

sourcery-ai Bot commented Sep 2, 2025

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactor TextEdit::updateMark to leverage QSet and removeIf for mark removal, eliminating manual index adjustments and adding debug logging for removed and remaining marks.

Class diagram for updated mark removal logic in TextEdit

classDiagram
    class TextEdit {
        +void updateMark(int from, int charsRemoved, int charsAdded)
        -QList<Selection> m_wordMarkSelections
    }
    class QSet {
        +contains(int)
    }
    TextEdit --> QSet : uses for mark removal
    TextEdit : updateMark now uses QSet and removeIf
    TextEdit : m_wordMarkSelections.removeIf(predicate)
Loading

Flow diagram for optimized mark removal in updateMark

flowchart TD
    A[Start updateMark] --> B[Identify marks to remove]
    B --> C{Is listRemoveItem empty?}
    C -- No --> D[Create QSet from listRemoveItem]
    D --> E[Iterate m_wordMarkSelections with index]
    E --> F[Remove selection if index in QSet]
    F --> G[Count removed marks]
    G --> H[Log removed and remaining marks]
    C -- Yes --> I[Skip removal]
    H --> J[End]
    I --> J
Loading

File-Level Changes

Change Details Files
Optimize mark removal logic using QSet and removeIf
  • Add include
  • Build QSet from listRemoveItem
  • Use removeIf with lambda instead of manual loop and removeAt
  • Insert qDebug output for removedCount and size
src/editor/dtextedit.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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • The PR description mentions file path handling enhancements in Window, but no related changes are included here—please add those updates or adjust the description to match.
  • Consider replacing the raw qDebug() with a qCDebug() category (or guard it behind a verbosity flag) to avoid flooding logs in production builds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The PR description mentions file path handling enhancements in Window, but no related changes are included here—please add those updates or adjust the description to match.
- Consider replacing the raw qDebug() with a qCDebug() category (or guard it behind a verbosity flag) to avoid flooding logs in production builds.

## Individual Comments

### Comment 1
<location> `src/editor/dtextedit.cpp:6514` </location>
<code_context>
+            int removedCount = m_wordMarkSelections.removeIf([&removeSet, &index](const auto&) {
+                return removeSet.contains(index++);
+            });
+            qDebug() << "updateMark: Removed" << removedCount << "marks, remaining:" << m_wordMarkSelections.size();
         }
     }
</code_context>

<issue_to_address>
Debug logging may be left in production code.

Consider removing or conditionally enabling this qDebug statement to avoid unnecessary log output and exposure of internal details in production.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
            qDebug() << "updateMark: Removed" << removedCount << "marks, remaining:" << m_wordMarkSelections.size();
=======
#ifdef QT_DEBUG
            qDebug() << "updateMark: Removed" << removedCount << "marks, remaining:" << m_wordMarkSelections.size();
#endif
>>>>>>> REPLACE

</suggested_fix>

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.

Comment thread src/editor/dtextedit.cpp
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, 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

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/forcemerge

@deepin-bot deepin-bot Bot merged commit ebf6f7e into linuxdeepin:master Sep 2, 2025
20 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