Skip to content

Terminal: clear a stale wrap mark, and copy and paint a selection whole - #2905

Merged
akurtakov merged 1 commit into
eclipse-platform:masterfrom
insjang:selection-copy
Nov 6, 2026
Merged

akurtakov merged 1 commit into
eclipse-platform:masterfrom
insjang:selection-copy

Conversation

@insjang

@insjang insjang commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Depends on #2894 (East Asian Width) for the filler handling in the copy path. This PR shows that commit as well until #2894 is merged; only the second commit is new here.

  • Wrap mark comes off. The mark that says a line runs on to the next (set when the terminal folds a line at the margin) could only be set, never cleared. A program that draws its own screen writes the same row over and over, and a row folded in one frame is a row of its own in the next; the stale mark then joined unrelated rows when copying. The mark now comes off when a row is drawn over shorter than the margin, and on erase to end of line. ITerminalTextData.clearWrappedLine is added as a default method (@since 1.2, bundle 1.2.0).
  • Copy without the room a fold left. Copying a folded line included the never-written cells past its last character — on a folded line the room the fold left, on an ended one the rest of the screen. They are left out, so a folded line copies as the one line it is.
  • Selection painted whole. The selection colour was drawn only where text was drawn, leaving cells past the last character and gaps under narrow glyphs in ordinary colours. The selected range on each line is filled first.
  • Narrower grid redraws. The canvas uses NO_REDRAW_RESIZE; when the grid got narrower, what was drawn in the columns now past its edge stayed on screen.
  • Drag past the edge keeps scrolling, so a selection can reach beyond the visible page.

Tests: wrap mark on/off in VT100EmulatorBackendTest; clearWrappedLine in AbstractITerminalTextDataTest for every data implementation.

@insjang insjang changed the title Terminal: keep the wrap mark honest, copy a folded line whole, paint a selection whole, and scroll while a drag goes past the edge Terminal: clear a stale wrap mark, and copy and paint a selection whole Sep 3, 2026
@insjang
insjang force-pushed the selection-copy branch 2 times, most recently from 08f9168 to 1e5d1de Compare September 30, 2026 08:06
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    54 files  ±0      54 suites  ±0   39m 54s ⏱️ +20s
 4 990 tests +3   4 968 ✅ +3   22 💤 ±0  0 ❌ ±0 
12 861 runs  +9  12 707 ✅ +9  154 💤 ±0  0 ❌ ±0 

Results for commit 5d8e26a. ± Comparison against base commit 14b3c15.

♻️ This comment has been updated with latest results.

@insjang

insjang commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on master, now that #2894 is in: one commit, 3f96987; all terminal tests pass.

How to test, in a runtime Eclipse with this org.eclipse.terminal.control, Terminal view, local shell:

  1. Wrap mark comes off: in a view 40 columns wide, run htop (or watch -n1 date in a narrow view), make it redraw for a few seconds, quit, then select and copy several of its last rows.
    Expected: one copied line per row. Before this change rows that had once been folded were joined.
  2. Copy without the room a fold left: printf '%050d\n' 0 in a 40 column view, select both rows and copy.
    Expected: one line of 50 zeros with no spaces after it; and copying a short line gives no trailing spaces.
  3. Selection painted whole: select a few lines that end at different columns, including text with narrow glyphs.
    Expected: the selection colour is a solid block over the selected cells, past the end of the text too.
  4. Narrower grid redraws: fill the screen, then make the view narrower by dragging its edge.
    Expected: nothing stays drawn past the new right edge.
  5. Drag past the edge: with a long history, drag a selection above the top of the view and hold the pointer there.
    Expected: the view keeps scrolling up and the selection grows with it.

Unit tests: testWrappedLineMarkComesOff in VT100EmulatorBackendTest; clearWrappedLine in AbstractITerminalTextDataTest and TerminalTextDataWindowTest for every data implementation.

@insjang
insjang force-pushed the selection-copy branch 3 times, most recently from 436f1fc to 4227a36 Compare November 5, 2026 07:19
@akurtakov
akurtakov requested a balanced review from Copilot November 5, 2026 13:11

Copilot AI 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.

🟡 Changes recommended

Outstanding defects drop selected whitespace, add incorrect newlines after partial overwrites, break word/line selection during edge dragging, and leave snapshots with stale wrap markers.

4 open issues
What changed in this PR

This PR improves terminal wrap tracking, copying, and selection display, building on the East Asian character-width support in #2894.

Changes:

  • Adds clearWrappedLine across terminal data implementations, with emulator and model tests.
  • Trims copied selection text and fills the full selected cell range.
  • Adds continuous scrolling during selection drags and redraws when the grid narrows.
File Description
terminal/​tests/​org.eclipse.terminal.test/​src/​org/​eclipse/​terminal/​internal/​model/​TerminalTextDataWindowTest.java Tests clearing wrap marks inside and outside the window.
terminal/​tests/​org.eclipse.terminal.test/​src/​org/​eclipse/​terminal/​internal/​model/​AbstractITerminalTextDataTest.java Tests wrap-mark clearing across data implementations.
terminal/​tests/​org.eclipse.terminal.test/​src/​org/​eclipse/​terminal/​internal/​emulator/​VT100EmulatorBackendTest.java Tests clearing wrap marks after overwrites and erasure.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​model/​ITerminalTextData.java Adds the default clearWrappedLine API.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​textcanvas/​TextLineRenderer.java Fills the full selection background before drawing text.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​textcanvas/​TextCanvas.java Adds drag scrolling and redraws after grid narrowing.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​textcanvas/​AbstractTextCanvasModel.java Trims trailing whitespace from copied selection rows.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​model/​TerminalTextDataWindow.java Delegates wrap clearing for rows within the window.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​model/​TerminalTextDataStore.java Clears the stored wrap flag.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​model/​TerminalTextDataFastScroll.java Maps wrap clearing to the underlying row position.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​model/​TerminalTextData.java Delegates wrap clearing to its backing data.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​model/​SynchronizedTerminalTextData.java Provides synchronized wrap clearing.
terminal/​bundles/​org.eclipse.terminal.control/​src/​org/​eclipse/​terminal/​internal/​emulator/​VT100EmulatorBackend.java Clears wrap marks on short writes and erase-to-end operations.

Review effort: Balanced

@insjang
insjang force-pushed the selection-copy branch 3 times, most recently from 42fc4eb to afcde2d Compare November 6, 2026 01:28
@akurtakov
akurtakov requested a balanced review from Copilot November 6, 2026 05:11

Copilot AI 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.

🟡 Changes recommended

Stale drag state can scroll a new selection incorrectly, while wide-character erasure can leave an invalid wrap marker that causes copied lines to be joined.

2 open issues
4 resolved since last review

Review effort: Balanced

The mark that says a line runs on to the next (set when the terminal
folds a line at the margin) could only be set, never cleared. A program
that draws its own screen erases a row before it draws it again, and a
row that was folded in one frame is a row of its own in the next; the
stale mark then joined unrelated rows when copying. The mark now comes
off when what ran on to the margin is erased or moved off it: erasing
to the end of the line, as xterm and Ghostty do, erasing characters as
far as the margin (Ghostty clears it on any such erase), or deleting
characters (xterm). Writing over part of the line leaves it, since the
line still runs on. Setting or clearing the mark tells snapshots,
so a reader sees it without another change to the line.
ITerminalTextData gains clearWrappedLine, a default method.

The selection colour was drawn only where text was drawn, leaving the
cells past the last character on a line and the gaps under narrow
glyphs in the ordinary colours. The selected range on each line is now
filled first.

The canvas is created with NO_REDRAW_RESIZE, which repaints only what a
resize uncovers; when the grid got narrower, what was drawn in the
columns now past its edge stayed on screen until something else
repainted it. A narrower grid now redraws. And a drag that goes past
the top or bottom edge keeps scrolling while the pointer stays there,
so a selection can reach beyond the visible page, by characters, words
or lines as the drag began.

ITerminalTextData.clearWrappedLine is a new default method
(@SInCE 1.2), within the 1.2.0 bundle and the 1.1.0
org.eclipse.terminal.model export that master already has.
@akurtakov

Copy link
Copy Markdown
Member

Drag past the edge keeps scrolling is a very nice improvement. In the future when you open new PRs make them as small as possible - e.g. this one fixes 5 things but should be one.

@akurtakov
akurtakov merged commit bc125e3 into eclipse-platform:master Nov 6, 2026
17 checks passed
@insjang

insjang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the review and the merge. Understood: from now on each new PR will do one thing and be as small as I can make it. I will not open new ones while #2902, #2904 and #3059 are under review, and later work will come as a queue of small, single-purpose PRs.

insjang added a commit to insjang/eclipse-modern-terminal that referenced this pull request Nov 7, 2026
… (EL 0, ECH to the margin), not when part of the line is written over; setting or clearing it tells snapshots; copying keeps written trailing spaces; a drag past the edge extends the selection by characters, words or lines as it began. As reviewed for eclipse-platform/eclipse.platform#2905.
insjang added a commit to insjang/eclipse-modern-terminal that referenced this pull request Nov 7, 2026
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