fix: resolve multi-line unindent issue with Shift+Tab#424
Merged
wyu71 merged 1 commit intolinuxdeepin:masterfrom Feb 27, 2026
Merged
fix: resolve multi-line unindent issue with Shift+Tab#424wyu71 merged 1 commit intolinuxdeepin:masterfrom
wyu71 merged 1 commit intolinuxdeepin:masterfrom
Conversation
- Added UnindentTextCommand class to handle multi-line unindentation - Fixed unindentText() to detect and process multi-line selections - Added boundary checks to prevent position out of range warnings - Preserved text selection state after unindent operation Log: resolve multi-line unindent issue with Shift+Tab pms: BUG-351091
deepin pr auto review这段代码实现了一个多行文本取消缩进（Unindent）的功能，主要通过新增 1. 语法逻辑与代码逻辑1.1
|
Reviewer's GuideAdds a new UnindentTextCommand to support multi-line unindent via Shift+Tab, refactors TextEdit::unindentText() to detect selections and delegate appropriately while preserving cursor/selection state, and updates license headers. Sequence diagram for multi-line ShiftTab unindent handlingsequenceDiagram
actor User
participant TextEdit
participant QUndoStack
participant UnindentTextCommand
User->>TextEdit: trigger ShiftTab
activate TextEdit
TextEdit->>TextEdit: unindentText()
TextEdit->>TextEdit: cursor = textCursor()
alt cursor hasSelection
TextEdit->>TextEdit: compute pos1, pos2, line1, line2
TextEdit->>UnindentTextCommand: new UnindentTextCommand(this,pos1,pos2,line1,line2,m_tabSpaceNumber)
TextEdit->>QUndoStack: push(UnindentTextCommand)
activate QUndoStack
QUndoStack->>UnindentTextCommand: redo()
deactivate QUndoStack
else no selection
TextEdit->>TextEdit: move cursor to StartOfBlock
TextEdit->>TextEdit: select first char
alt first char is tab
TextEdit->>DeleteBackCommand: new DeleteBackCommand(cursor,this)
TextEdit->>QUndoStack: push(DeleteBackCommand)
activate QUndoStack
QUndoStack->>DeleteBackCommand: redo()
deactivate QUndoStack
else first char is space
TextEdit->>TextEdit: count spaces up to m_tabSpaceNumber
TextEdit->>TextEdit: select spaces
TextEdit->>DeleteBackCommand: new DeleteBackCommand(cursor,this)
TextEdit->>QUndoStack: push(DeleteBackCommand)
activate QUndoStack
QUndoStack->>DeleteBackCommand: redo()
deactivate QUndoStack
end
end
TextEdit-->>User: text unindented
Updated class diagram for text unindent commandsclassDiagram
class QUndoCommand
class QUndoStack
class QTextCursor
class QVector_int
class TextEdit {
int m_tabSpaceNumber
QUndoStack* m_pUndoStack
QTextCursor textCursor()
void setTextCursor(QTextCursor cursor)
void unindentText()
}
class IndentTextCommand {
+IndentTextCommand(TextEdit* edit)
+~IndentTextCommand()
+void redo()
+void undo()
}
class UnindentTextCommand {
+UnindentTextCommand(TextEdit* edit,int startpos,int endpos,int startline,int endline,int tabSpaceNumber)
+~UnindentTextCommand()
+void redo()
+void undo()
TextEdit* m_edit
int m_startpos
int m_endpos
int m_startline
int m_endline
bool m_hasselected
int m_tabSpaceNumber
QVector_int m_removedChars
}
class DeleteBackCommand {
+DeleteBackCommand(QTextCursor cursor,TextEdit* edit)
+void redo()
+void undo()
}
QUndoCommand <|-- IndentTextCommand
QUndoCommand <|-- UnindentTextCommand
QUndoCommand <|-- DeleteBackCommand
TextEdit --> QUndoStack : uses m_pUndoStack
TextEdit --> UnindentTextCommand : creates for multiline
TextEdit --> DeleteBackCommand : creates for single line
UnindentTextCommand --> TextEdit : operates on
UnindentTextCommand --> QVector_int : tracks removed chars
UnindentTextCommand --> QTextCursor : local cursor operations
DeleteBackCommand --> TextEdit : operates on
DeleteBackCommand --> QTextCursor
IndentTextCommand --> TextEdit : operates on
IndentTextCommand --> QTextCursor
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new UnindentTextCommand contains quite a lot of qDebug/qInfo logging for constructor, destructor, redo, and undo, which will fire very frequently for Shift+Tab; consider reducing or guarding these logs to avoid noisy output in normal usage.
- The unindent logic is now split between UnindentTextCommand for selections and the old DeleteBackCommand path for single lines; consider routing the single-line case through UnindentTextCommand as well to keep behavior consistent and reduce duplicated indentation logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new UnindentTextCommand contains quite a lot of qDebug/qInfo logging for constructor, destructor, redo, and undo, which will fire very frequently for Shift+Tab; consider reducing or guarding these logs to avoid noisy output in normal usage.
- The unindent logic is now split between UnindentTextCommand for selections and the old DeleteBackCommand path for single lines; consider routing the single-line case through UnindentTextCommand as well to keep behavior consistent and reduce duplicated indentation logic.
## Individual Comments
### Comment 1
<location path="src/editor/indenttextcommond.cpp" line_range="123-131" />
<code_context>
+ for(int i=m_startline;i<=m_endline;i++){
+ cursor.movePosition(QTextCursor::StartOfBlock);
+ int lineStart = cursor.position();
+ int removed = 0;
+
+ //check if line starts with tab
+ if(m_edit->document()->characterAt(lineStart) == '\t'){
+ cursor.deleteChar();
+ removed = 1;
+ }
+ //check if line starts with spaces
+ else if(m_edit->document()->characterAt(lineStart) == ' '){
+ int cnt = 0;
+ int pos = lineStart;
</code_context>
<issue_to_address>
**issue (bug_risk):** Undo logic can restore a tab when the original indentation was a single space.
In `redo()`, lines starting with spaces set `removed = cnt` (1..m_tabSpaceNumber), but in `undo()` the `removed == 1` case is treated as a tab and restores `"\t"`. So a line that originally had a single leading space is restored with a tab. Instead of inferring from `removed`, you should record whether each line’s removed indentation was a tab or spaces (e.g., a parallel `QVector<bool>` or small enum).
</issue_to_address>
### Comment 2
<location path="src/editor/indenttextcommond.cpp" line_range="152-160" />
<code_context>
+ }
+
+ //reset selection.
+ if(m_hasselected){
+ qDebug() << "UnindentTextCommand redo, m_hasselected, totalRemoved:" << totalRemoved;
+ if(m_startline == m_endline){
+ qDebug() << "UnindentTextCommand redo, m_startline == m_endline";
+ int newStartPos = m_startpos - m_removedChars[0];
+ if(newStartPos < 0) newStartPos = 0;
+ cursor.setPosition(newStartPos);
+ cursor.movePosition(QTextCursor::StartOfBlock);
+ cursor.movePosition(QTextCursor::EndOfBlock,QTextCursor::KeepAnchor);
+ m_edit->setTextCursor(cursor);
+ }
</code_context>
<issue_to_address>
**question (bug_risk):** Single-line selection is expanded to the full line after unindent redo.
In the single-line case, `redo()` always selects the full block, ignoring the original selection range. Previously, unindent preserved the selection on that line, so this is a behavior change that may confuse users. If you want to keep the original range, you could adjust `[m_startpos, m_endpos]` by the removed count for that line rather than reselecting the entire block.
</issue_to_address>
### Comment 3
<location path="src/editor/indenttextcommond.cpp" line_range="90" />
<code_context>
qDebug() << "IndentTextCommand undo exit";
}
+
+UnindentTextCommand::UnindentTextCommand(TextEdit* edit,int startpos,int endpos,int startline,int endline,int tabSpaceNumber):
+ m_edit(edit),
+ m_startpos(startpos),
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring UnindentTextCommand to be anchored on a stored QTextCursor and QTextBlock range rather than multiple position/line members and the live editor cursor.
You can reduce a lot of the positional/state complexity by anchoring the command around a stored `QTextCursor` and iterating `QTextBlock`s directly, instead of juggling `m_startpos`/`m_endpos`/`m_startline`/`m_endline` and the editor’s live cursor.
Concretely:
1. **Store a private cursor and block range in the command**
```cpp
class UnindentTextCommand : public QUndoCommand {
public:
UnindentTextCommand(TextEdit *edit, int tabSpaceNumber)
: m_edit(edit)
, m_cursor(edit->textCursor())
, m_tabSpaceNumber(tabSpaceNumber)
{
m_hasselected = m_cursor.hasSelection();
m_cursor.setPosition(m_cursor.selectionStart());
m_firstBlock = m_cursor.block();
m_cursor.setPosition(m_cursor.selectionEnd());
m_lastBlock = m_cursor.block();
const int blockCount = blockDistance(m_firstBlock, m_lastBlock) + 1;
m_removedChars.resize(blockCount);
}
private:
static int blockDistance(const QTextBlock &from, const QTextBlock &to) {
int n = 0;
for (QTextBlock b = from; b != to; b = b.next())
++n;
return n;
}
TextEdit *m_edit;
QTextCursor m_cursor;
QTextBlock m_firstBlock;
QTextBlock m_lastBlock;
QVector<int> m_removedChars;
bool m_hasselected = false;
int m_tabSpaceNumber = 4;
};
```
This removes the need for `m_startpos`, `m_endpos`, `m_startline`, `m_endline` and avoids depending on `m_edit->textCursor()` in `redo()`/`undo()`.
2. **Redo: iterate blocks and work from `block.position()`**
```cpp
void UnindentTextCommand::redo()
{
int totalRemoved = 0;
int index = 0;
for (QTextBlock block = m_firstBlock;
block.isValid() && block.position() <= m_lastBlock.position();
block = block.next(), ++index)
{
QTextCursor c(block);
const int lineStart = block.position();
int removed = 0;
const QChar ch = m_edit->document()->characterAt(lineStart);
if (ch == '\t') {
c.deleteChar();
removed = 1;
} else if (ch == ' ') {
int pos = lineStart;
int cnt = 0;
while (m_edit->document()->characterAt(pos) == ' ' && cnt < m_tabSpaceNumber) {
++pos;
++cnt;
}
if (cnt > 0) {
c.setPosition(lineStart);
c.setPosition(pos, QTextCursor::KeepAnchor);
c.removeSelectedText();
removed = cnt;
}
}
m_removedChars[index] = removed;
totalRemoved += removed;
}
if (m_hasselected) {
// recompute selection from block positions + removed deltas instead of raw offsets
QTextCursor selCursor(m_firstBlock);
const int startOffset = m_removedChars.isEmpty() ? 0 : m_removedChars.first();
selCursor.setPosition(m_firstBlock.position() - startOffset);
QTextBlock afterLast = m_lastBlock.next();
int endPos = afterLast.isValid() ? afterLast.position() : m_edit->document()->characterCount();
endPos -= totalRemoved;
selCursor.setPosition(endPos, QTextCursor::KeepAnchor);
m_edit->setTextCursor(selCursor);
}
}
```
Key changes:
- Work per `QTextBlock` instead of line indexes and repeatedly resetting a shared cursor.
- Selection restoration is derived from `block.position()` and aggregated `removed` values, not manual `m_startpos`/`m_endpos` arithmetic.
3. **Undo: symmetric block-based restore**
```cpp
void UnindentTextCommand::undo()
{
int index = 0;
for (QTextBlock block = m_firstBlock;
block.isValid() && block.position() <= m_lastBlock.position();
block = block.next(), ++index)
{
QTextCursor c(block);
const int removed = m_removedChars.value(index);
if (removed == 1) {
c.insertText("\t");
} else if (removed > 1) {
c.insertText(QString(removed, ' '));
}
}
if (m_hasselected) {
// just restore original stored cursor selection
m_edit->setTextCursor(m_cursor);
}
}
```
This keeps all functionality (multi-line unindent with undo, per-line removed-char tracking) while:
- Eliminating the explicit `m_startpos`/`m_endpos`/`m_startline`/`m_endline` state.
- Decoupling the command from whatever the widget’s cursor happens to be at call time.
- Removing low-level selection offset math in favor of block-based computation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
lzwind
approved these changes
Feb 27, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, wyu71 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: resolve multi-line unindent issue with Shift+Tab
pms: BUG-351091
Summary by Sourcery
Handle unindenting for both single-line and multi-line text selections while preserving undo/redo behavior.
Bug Fixes:
Enhancements: