Skip to content

fix: merge column-edit undo commands for single-step undo - #688

Open
pppanghu77 wants to merge 1 commit into
linuxdeepin:release/eaglefrom
pppanghu77:fix/254289-column-edit-undo-merge
Open

pppanghu77 wants to merge 1 commit into
linuxdeepin:release/eaglefrom
pppanghu77:fix/254289-column-edit-undo-merge

Conversation

@pppanghu77

@pppanghu77 pppanghu77 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Fix: Column-edit undo merges consecutive insert commands (PMS BUG-254289)

Problem

In column-edit mode (ALT+mouse), typing multiple characters then pressing Ctrl+Z undoes one character at a time instead of the entire input. This is because InsertTextUndoCommand does not override id()/mergeWith(), so QUndoStack never merges consecutive single-char insert commands.

Root Cause

InsertTextUndoCommand lacks id() and mergeWith() overrides. Each keypress in column-edit mode creates a separate InsertTextUndoCommand pushed to the undo stack. Without id()/mergeWith(), QUndoStack treats each as independent, requiring one undo per character.

Fix

  • Add id() returning 1001 and mergeWith() to InsertTextUndoCommand
  • mergeWith() appends m_sInsertText only when both commands have non-empty m_ColumnEditSelections (isolates non-column-edit scenarios)
  • Fix undo() column-edit branch to delete m_sInsertText.length() chars instead of single deleteChar() (necessary after merge since m_sInsertText is now multi-char)

Verification

Minimal Qt6 demo validated: without mergeWith(), typing "//" requires 2 undo steps; with mergeWith(), 1 undo step removes both characters.

Impact

  • Column-edit undo: single-step undo for consecutive input
  • Normal text insert: unaffected (m_ColumnEditSelections is empty, mergeWith returns false)
  • No other QUndoCommand subclass uses id(), so 1001 has no conflict

PMS: BUG-254289

Summary by Sourcery

Enable single-step undo for consecutive text input in column-edit mode while keeping normal text insertion behavior unchanged.

Bug Fixes:

  • Merge consecutive column-edit text insertions into a single undo step.
  • Preserve correct multi-character deletion when undoing merged column-edit input.
  • Prevent undo merging across separate column-edit sessions.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pppanghu77

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

@sourcery-ai

sourcery-ai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Reviewer's Guide

Updates InsertTextUndoCommand to merge consecutive column-edit character insertions and removes the combined text in a single undo operation, while leaving normal text insertion commands unmerged.

Sequence diagram for merged column-edit undo

sequenceDiagram
    actor User
    participant Edit as QPlainTextEdit
    participant Stack as QUndoStack
    participant Cmd as InsertTextUndoCommand

    User->>Edit: Type character
    Edit->>Cmd: InsertTextUndoCommand
    Edit->>Stack: push(Cmd)
    User->>Edit: Type next character
    Edit->>Cmd: InsertTextUndoCommand
    Edit->>Stack: push(Cmd)
    Stack->>Cmd: id()
    Stack->>Cmd: mergeWith(other)
    Cmd-->>Stack: true, append m_sInsertText
    User->>Stack: undo()
    Stack->>Cmd: undo()
    Cmd->>Cmd: delete combined m_sInsertText
Loading

Flow diagram for column-edit command merging

flowchart TD
    A[InsertTextUndoCommand pushed] --> B{"id() matches 1001?"}
    B -- No --> C[Keep separate undo command]
    B -- Yes --> D{Both m_ColumnEditSelections non-empty?}
    D -- No --> C
    D -- Yes --> E[mergeWith appends m_sInsertText]
    E --> F[One undo removes merged text]
Loading

File-Level Changes

Change Details Files
Merge consecutive column-edit insert undo commands into one undo step.
  • Added a shared command ID and QUndoCommand mergeWith() override.
  • Merged insert text only when both commands represent column-edit operations.
  • Preserved normal insert behavior by rejecting merges without column selections.
src/editor/inserttextundocommand.h
src/editor/inserttextundocommand.cpp
Make merged column-edit commands undo the complete inserted text.
  • Changed column-edit undo to select the full merged text length before deleting it.
  • Retained cursor and edit refresh handling after deletion.
src/editor/inserttextundocommand.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 - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@pppanghu77
pppanghu77 force-pushed the fix/254289-column-edit-undo-merge branch from 6ef56d9 to c5945a1 Compare October 23, 2026 10:38
1. Root cause: InsertTextUndoCommand lacked id()/mergeWith() overrides,
   so QUndoStack never merged consecutive single-char column-edit insert
   commands; each keypress produced a separate undo step. Additionally,
   mergeWith() had no session continuity check, causing cross-session
   incorrect merging of column-edit undo commands
2. Fix: add id() returning kColumnEditInsertCommandId and mergeWith()
   that appends text when both commands have non-empty
   m_ColumnEditSelections and share the same m_sessionId; add
   m_columnEditSessionId member to TextEdit, increment it in
   mousePressEvent at session boundaries (Alt+click enter and
   non-Alt click exit); pass session ID to InsertTextUndoCommand
   constructor; update m_ColumnEditSelections after merge so undo
   operates on latest cursor positions; fix redo() selection to use
   m_sInsertText.length() without +1
3. Impact: column-edit undo now removes entire typed text in one step
   within the same session; cross-session typing is not incorrectly
   merged; redo path correct after merge; normal text insert unaffected
   (m_ColumnEditSelections is empty, so mergeWith returns false)

Log: fix column-edit undo to remove all typed text in a single undo step

Influence:
1. Test column-edit mode: type multiple chars then Ctrl+Z, verify
   single-step undo
2. Test column-edit redo after undo: Ctrl+Shift+Z then Ctrl+Z again,
   verify no data corruption
3. Test cross-session column-edit: enter column-edit, type, exit,
   re-enter column-edit, type again, verify undo does not merge
   across sessions
4. Test normal text editing undo/redo remains unchanged

fix: 修复列编辑模式撤销需逐字符撤销的问题

1. 根因:InsertTextUndoCommand 未重写 id()/mergeWith(),QUndoStack
   无法合并列编辑模式下连续按键产生的单字符插入命令,每次按键产生
   独立的撤销步骤;此外 mergeWith() 无会话连续性判断,导致跨会话
   错误合并列编辑撤销命令
2. 方案:添加 id() 返回 kColumnEditInsertCommandId 和 mergeWith(),
   当双方 m_ColumnEditSelections 均非空且 m_sessionId 相同时合并文本;
   在 TextEdit 中添加 m_columnEditSessionId 成员,在 mousePressEvent
   会话边界(Alt+点击进入和非 Alt 点击退出)递增;将 session ID 传入
   InsertTextUndoCommand 构造函数;合并后更新 m_ColumnEditSelections
   使撤销基于最新光标位置;修复 redo() 选区计算移除 +1
3. 影响:列编辑撤销现在可在同一会话内一步撤销全部输入;跨会话输入
   不会被错误合并;redo 路径在合并后同样正确;普通文本编辑不受影响
   (m_ColumnEditSelections 为空,mergeWith 返回 false)

Log: 修复列编辑模式撤销需逐字符撤销的问题

Influence:
1. 测试列编辑模式:输入多个字符后 Ctrl+Z,验证一步撤销
2. 测试列编辑撤销后重做再撤销:Ctrl+Shift+Z 后 Ctrl+Z,验证无数据损坏
3. 测试跨会话列编辑:进入列编辑输入,退出后重新进入列编辑再输入,
   验证撤销不会跨会话合并
4. 测试普通文本编辑的撤销/重做行为保持不变

PMS: BUG-254289
@pppanghu77
pppanghu77 force-pushed the fix/254289-column-edit-undo-merge branch from c5945a1 to 734912a Compare October 28, 2026 10:08
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 98 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 本次提交实现了列编辑模式下撤销命令的合并功能,通过引入 session ID 机制确保同一编辑会话内的连续输入可单步撤销。代码逻辑正确,修复了 redo() 中的 off-by-one 偏移问题,undo() 操作改为精确定位删除。无安全漏洞,性能无影响。仅 mergeWith 关键函数缺少注释说明。

🔍 详细分析

1. 语法逻辑 ✅

评价: 语法正确,逻辑清晰 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['无语法逻辑问题']


2. 代码质量 ✅

评价: 代码结构清晰,注释基本完整 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['建议为 mergeWith() 添加简短注释,说明合并条件(同一 session、同一命令类型)和合并策略(文本拼接、选区更新)']


3. 代码性能 ✅

评价: 性能良好,资源使用合理 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['无性能优化建议']


4. 代码安全 🔒

评价: 存在0个安全漏洞 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: ['无安全加固建议']


💡 改进建议代码示例

// 建议为 mergeWith 添加注释说明合并策略
bool InsertTextUndoCommand::mergeWith(const QUndoCommand *other)
{
    // 仅合并同一命令类型、同一列编辑会话的命令
    if (other->id() != id())
        return false;

    const auto *cmd = static_cast<const InsertTextUndoCommand *>(other);
    if (m_ColumnEditSelections.isEmpty() || cmd->m_ColumnEditSelections.isEmpty())
        return false;

    if (m_sessionId != cmd->m_sessionId)
        return false;

    // 合并策略:拼接文本,更新选区为最新命令的选区
    m_sInsertText += cmd->m_sInsertText;
    m_ColumnEditSelections = cmd->m_ColumnEditSelections;
    return true;
}

本报告由 AI 代码审查工具自动生成

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.

2 participants