Skip to content

Drop a line's shell integration marks when ED erases it whole - #155

Merged
JohnCampionJr merged 2 commits into
tomlm:mainfrom
JohnCampionJr:fix/ed-drops-marks
Sep 4, 2026
Merged

Drop a line's shell integration marks when ED erases it whole#155
JohnCampionJr merged 2 commits into
tomlm:mainfrom
JohnCampionJr:fix/ed-drops-marks

Conversation

@JohnCampionJr

Copy link
Copy Markdown
Collaborator

What

clear is CSI 3 J, CUP, CSI 2 J: the scrollback discarded, the screen blanked in place. ED 3 takes the lines and their marks with it, but ED 2 blanked the screen rows and left their OSC 133 marks behind — so a host drawing a gutter from them kept painting prompt and exit bars beside empty rows. Seen with pwsh's Clear-Host in the Iciclecreek demo's gutter.

Fix

A row ED erases in full now loses its marks along with its double-size attribute, through the same nothing-survived decision already in EraseLineCells. The rules:

  • ED, whole row, nothing survived → marks dropped (new).
  • EL keeps marks, as before: shells redraw a prompt they have just marked with EL.
  • Cursor row of ED 0 / ED 1 goes through EL and keeps its mark, so a shell that homes the cursor and erases below before printing its prompt (zsh after a redraw) keeps that prompt's mark.
  • Selective erases leave marks standing with the text they protect.

Kitty and Ghostty both drop a row's prompt flag when ED clears it, so this matches the terminals hosts will be compared against.

Tests

Three added to ShellIntegrationMarkAnchorTests: the clear sequence drops every row's marks; erase-below keeps the cursor row's mark and drops the rest; a selective screen erase leaves marks alone. Full suite: 2217 passed, 1 skipped.

🤖 Generated with Claude Code

`clear` is CSI 3 J, CUP, CSI 2 J: the scrollback discarded, the screen
blanked in place. ED 3 takes the lines and their marks with it, but ED 2
blanked the screen rows and left their OSC 133 marks behind, so a host
drawing a gutter from them kept painting prompt and exit bars beside
empty rows.

A row ED erases in full now loses its marks along with its double-size
attribute, through the same nothing-survived decision. EL keeps them,
as before: shells redraw a prompt they have just marked with EL. The
cursor row of ED 0 and ED 1 goes through EL, so a shell that homes the
cursor and erases below before printing its prompt keeps that mark.
Selective erases leave marks standing with the text they protect.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The behavior is covered by regression tests, with only a non-blocking documentation nit remaining.

Pull request overview

Updates full-row display erases to remove stale shell-integration marks while preserving marks for EL and selective erases.

Changes:

  • Clears marks when a row is fully erased.
  • Adds regression coverage for screen-erasure variants.
  • Non-blocking nit: broaden affected XML summaries beyond ED-only behavior.
File summaries
File Description
src/XTerm.NET/InputHandler.Protection.cs Clears marks alongside full-row attributes.
src/XTerm.NET.Tests/ShellIntegrationMarkAnchorTests.cs Tests mark retention and removal rules.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/XTerm.NET/InputHandler.Protection.cs
Copilot's review: ClearMarks claimed only line reuse called it, and the
helper described itself as ED-only when EraseWholeScreen -- DECCOLM's
clear, DECRST 1047 blanking the alternate screen -- reaches it too.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@JohnCampionJr
JohnCampionJr merged commit dad7a55 into tomlm:main Sep 4, 2026
5 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.

2 participants