fix(terminal): clamp DCH count to cells remaining from cursor#13265
Open
Jason-Shen2 wants to merge 1 commit into
Open
fix(terminal): clamp DCH count to cells remaining from cursor#13265Jason-Shen2 wants to merge 1 commit into
Jason-Shen2 wants to merge 1 commit into
Conversation
Fixes warpdotdev#12820. When CSI Pn P (DCH - Delete Character) received a count greater than the number of cells remaining from cursor to end-of-line, the count was clamped to the full row width (cols). This caused the trailing blank-fill loop to start at cols - count, which could land before the cursor column, erroneously erasing characters before the cursor. The fix mirrors the existing pattern in insert_blank: clamp count to cols - cursor.point.col so DCH never reaches cells before the cursor. Added a regression test to verify characters before the cursor are preserved when count exceeds cells to EOL.
github-actions
Bot
added
the
external-contributor
Contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Overview
This PR updates terminal DCH handling to clamp the delete count to the cells remaining from the cursor to the end of the row, and adds a regression test for an over-large delete count preserving cells before the cursor.
Concerns
- This is a user-facing terminal behavior change, but the PR does not include screenshots or a screen recording demonstrating the fix end to end. For this user-facing change, please include a screenshot or short recording showing an oversized DCH count preserving characters before the cursor while blanking cells at/after the cursor.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
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
1 participant
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.
Fixes #12820.
Problem
When DCH (CSI Pn P - Delete Character) receives a count greater than the number of cells remaining from the cursor to end-of-line, the count was clamped to the full row width (cols). This caused the trailing blank-fill loop (at cols - count) to start at an index before the cursor column, erroneously erasing characters before the cursor.
Root Cause
In ansi_handler.rs delete_chars(), count was clamped to min(count, cols) (full row width), then the blank-fill started at cols - count. When count > cols - cursor_col, the blank-fill start falls before the cursor, wiping cells that should be preserved.
Fix
Clamp count to cells remaining from cursor (matching the existing pattern in insert_blank): min(count, cols - cursor.point.col).
Verification
Added regression test test_delete_chars_count_exceeding_eol_does_not_erase_before_cursor.