Skip to content

Commit 141407c

Browse files
Merge pull request #1880 from contour-terminal/fix/tab-characters-lost
Fix tab characters (HT) intermittently lost in output
2 parents 56b43e6 + d1c0911 commit 141407c

File tree

5 files changed

+122
-10
lines changed

5 files changed

+122
-10
lines changed

.github/actions/spelling/allow/allow.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ Dbus
77
ENDCHAR
88
ENDFONT
99
ENDPROPERTIES
10+
FFFFFFFF
1011
FONTBOUNDINGBOX
1112
FreeBSD
1213
LASTEXITCODE
1314
OPENCONSOLE
1415
OpenBSD
16+
RTX
1517
SEMA
1618
STARTCHAR
1719
STARTFONT
@@ -32,6 +34,7 @@ copy'n'paste
3234
copyable
3335
crossfade
3436
dbus
37+
flamegraphs
3538
gha
3639
ilammy
3740
lazygit
@@ -55,4 +58,5 @@ tparam
5558
unk
5659
urg
5760
vswhere
61+
winget
5862
zypper

metainfo.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@
119119
<li>Fixes ~2s startup delay caused by synchronous Qt multimedia driver probing by deferring audio initialization to a background thread</li>
120120
<li>Fixes initial font sizing at fractional DPR on Wayland by preferring screen DPR over window DPR</li>
121121
<li>Fixes Kitty keyboard protocol: proper CapsLock/NumLock modifier reporting, correct modifier encoding per spec, and Unicode case folding for non-ASCII characters</li>
122+
<li>Fixes tab characters (HT) intermittently lost in terminal output due to bulk text parser skipping trailing C0 controls and tab cursor movement not filling intermediate cells with spaces (#1876)</li>
122123
<li>Replaces abrupt cell blink toggle with configurable smooth pulse animation via blink_style profile setting (classic/smooth/linger)</li>
123124
<li>Adds configurable crossfade transition between primary and alternate screens via screen_transition and screen_transition_duration profile settings</li>
124125
<li>Adds animated cursor movement between grid cells with ease-out interpolation via cursor_motion_animation_duration profile setting</li>

src/vtbackend/Screen.cpp

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,6 +1621,9 @@ void Screen<Cell>::moveCursorToNextTab()
16211621
// TODO: respect HTS/TBC
16221622

16231623
static_assert(TabWidth > ColumnCount(0));
1624+
1625+
auto columnsToAdvance = ColumnCount(0);
1626+
16241627
if (!_terminal->tabs().empty())
16251628
{
16261629
// advance to the next tab
@@ -1631,21 +1634,33 @@ void Screen<Cell>::moveCursorToNextTab()
16311634
auto const currentCursorColumn = logicalCursorPosition().column;
16321635

16331636
if (i < _terminal->tabs().size())
1634-
moveCursorForward(boxed_cast<ColumnCount>(_terminal->tabs()[i] - currentCursorColumn));
1637+
columnsToAdvance = boxed_cast<ColumnCount>(_terminal->tabs()[i] - currentCursorColumn);
16351638
else if (realCursorPosition().column < margin().horizontal.to)
1636-
moveCursorForward(boxed_cast<ColumnCount>(margin().horizontal.to - currentCursorColumn));
1639+
columnsToAdvance = boxed_cast<ColumnCount>(margin().horizontal.to - currentCursorColumn);
16371640
}
16381641
else
16391642
{
16401643
// default tab settings
16411644
if (realCursorPosition().column < margin().horizontal.to)
16421645
{
1643-
auto const n =
1646+
columnsToAdvance =
16441647
std::min((TabWidth - boxed_cast<ColumnCount>(_cursor.position.column) % TabWidth),
16451648
pageSize().columns - boxed_cast<ColumnCount>(logicalCursorPosition().column));
1646-
moveCursorForward(n);
16471649
}
16481650
}
1651+
1652+
if (columnsToAdvance > ColumnCount(0))
1653+
{
1654+
// Fill intermediate cells with spaces to ensure TrivialLineBuffer consistency.
1655+
// Without this, the cursor advances but the cell buffer doesn't reflect the gap,
1656+
// causing tabs to be visually lost during rendering.
1657+
auto const startCol = _cursor.position.column;
1658+
auto& line = currentLine();
1659+
for (auto const i: std::views::iota(0, unbox(columnsToAdvance)))
1660+
line.useCellAt(startCol + ColumnOffset(i)).write(_cursor.graphicsRendition, L' ', 1);
1661+
1662+
moveCursorForward(columnsToAdvance);
1663+
}
16491664
}
16501665

16511666
template <CellConcept Cell>
@@ -3305,6 +3320,11 @@ template <CellConcept Cell>
33053320
void Screen<Cell>::executeControlCode(char controlCode)
33063321
{
33073322
#if defined(LIBTERMINAL_LOG_TRACE)
3323+
// Flush any pending text trace before processing the control code.
3324+
// Without this, when the parser's bulk text optimization processes
3325+
// text → C0 → text inline (without leaving Ground state), writeTextEnd()
3326+
// is never called, causing gaps in the trace log.
3327+
writeTextEnd();
33083328
if (vtTraceSequenceLog)
33093329
vtTraceSequenceLog()(
33103330
"control U+{:02X} ({})", controlCode, to_string(static_cast<ControlCode::C0>(controlCode)));

src/vtbackend/Screen_test.cpp

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3978,6 +3978,78 @@ TEST_CASE("LS1 and LS0", "[screen]")
39783978
// TODO: SendDeviceAttributes
39793979
// TODO: SendTerminalId
39803980

3981+
TEST_CASE("HorizontalTab.FillsCellsWithSpaces", "[screen]")
3982+
{
3983+
// Verify that HT fills intermediate cells with space characters,
3984+
// not just moves the cursor. This ensures TrivialLineBuffer consistency.
3985+
auto mock = MockTerm { PageSize { LineCount(2), ColumnCount(20) } };
3986+
auto& screen = mock.terminal.primaryScreen();
3987+
3988+
mock.writeToScreen("A\tB");
3989+
3990+
// 'A' at col 0, tab advances to col 8 (default tab width), 'B' at col 8.
3991+
// Columns 1..7 should be filled with spaces, rendering correctly.
3992+
CHECK(screen.logicalCursorPosition().column == ColumnOffset(9));
3993+
CHECK("A B \n \n" == screen.renderMainPageText());
3994+
}
3995+
3996+
TEST_CASE("HorizontalTab.AfterBulkText", "[screen]")
3997+
{
3998+
// Write printable ASCII followed by HT followed by more text.
3999+
// This exercises the parseBulkText fast-path → C0 execute → more text path.
4000+
auto mock = MockTerm { PageSize { LineCount(2), ColumnCount(20) } };
4001+
auto& screen = mock.terminal.primaryScreen();
4002+
4003+
mock.writeToScreen("AB\tCD");
4004+
4005+
// "AB" occupies columns 0-1, tab advances to column 8, "CD" at columns 8-9
4006+
CHECK("AB CD \n \n" == screen.renderMainPageText());
4007+
CHECK(screen.logicalCursorPosition().column == ColumnOffset(10));
4008+
}
4009+
4010+
TEST_CASE("HorizontalTab.MultipleTabs", "[screen]")
4011+
{
4012+
// "A\tB\tC" should produce correctly spaced output with space-filled cells.
4013+
auto mock = MockTerm { PageSize { LineCount(2), ColumnCount(25) } };
4014+
auto& screen = mock.terminal.primaryScreen();
4015+
4016+
mock.writeToScreen("A\tB\tC");
4017+
4018+
// 'A' at col 0, tab to col 8, 'B' at col 8, tab to col 16, 'C' at col 16
4019+
CHECK("A B C \n \n" == screen.renderMainPageText());
4020+
CHECK(screen.logicalCursorPosition().column == ColumnOffset(17));
4021+
}
4022+
4023+
TEST_CASE("HorizontalTab.AtChunkBoundary", "[screen]")
4024+
{
4025+
// Force text+tab across chunk boundaries by using a small ptyReadBufferSize.
4026+
// The tab character should still be processed correctly even at a chunk boundary.
4027+
auto mock = MockTerm { PageSize { LineCount(2), ColumnCount(20) }, LineCount(0), 4 };
4028+
auto& screen = mock.terminal.primaryScreen();
4029+
4030+
mock.writeToScreen("ABC\tD");
4031+
4032+
// "ABC" at cols 0-2, tab to col 8, 'D' at col 8
4033+
CHECK("ABC D \n \n" == screen.renderMainPageText());
4034+
}
4035+
4036+
TEST_CASE("HorizontalTab.AfterScreenClear", "[screen]")
4037+
{
4038+
// After ED (Erase in Display), write text with tabs and verify correct rendering.
4039+
// This tests TrivialLineBuffer reset + tab interaction.
4040+
auto mock = MockTerm { PageSize { LineCount(2), ColumnCount(20) } };
4041+
auto& screen = mock.terminal.primaryScreen();
4042+
4043+
// Write initial content
4044+
mock.writeToScreen("Hello World");
4045+
// Clear screen (CSI 2 J) and cursor home (CSI H)
4046+
mock.writeToScreen("\033[2J\033[H");
4047+
// Write text with tab
4048+
mock.writeToScreen("X\tY");
4049+
4050+
CHECK("X Y \n \n" == screen.renderMainPageText());
4051+
}
4052+
39814053
// NOLINTEND(misc-const-correctness,readability-function-cognitive-complexity)
39824054

39834055
// NOLINTBEGIN(misc-const-correctness)

src/vtparser/Parser-impl.h

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -474,13 +474,28 @@ auto Parser<EventListener, TraceStateChanges>::parseBulkText(char const* begin,
474474

475475
if (_scanState.utf8.expectedLength == 0)
476476
{
477-
// This optimization is for the `cat`-people.
478-
// It further optimizes the throughput performance by bypassing
479-
// the FSM for the `(TEXT LF+)+`-case.
477+
// Process trailing C0 control characters inline, bypassing the FSM.
478+
// This handles the common (TEXT C0)+ pattern (e.g. text followed by LF, HT, CR)
479+
// with a significant throughput improvement (~50x for cat-like workloads).
480480
//
481-
// As of bench-headless, the performance increase is about 50x.
482-
if (input != end && *input == '\n')
483-
_eventListener.execute(*input++);
481+
// C0 Execute range: 0x00-0x17, 0x19, 0x1C-0x1F (bits in a 32-bit mask)
482+
// Excluded: 0x18 (CAN) and 0x1A (SUB) — trigger state transitions (Ground + Ignore)
483+
// Excluded: 0x1B (ESC) — transitions to Escape state
484+
constexpr auto C0ExecuteMask =
485+
uint32_t { 0xFFFFFFFF } & ~(1u << 0x18) & ~(1u << 0x1A) & ~(1u << 0x1B);
486+
auto const* current = _scanState.next;
487+
while (current != end)
488+
{
489+
auto const ch = static_cast<uint8_t>(*current);
490+
if (ch < 0x20 && (C0ExecuteMask & (1u << ch)) != 0)
491+
{
492+
_eventListener.execute(*current);
493+
++current;
494+
}
495+
else
496+
break;
497+
}
498+
_scanState.next = current;
484499
}
485500

486501
auto const count = static_cast<size_t>(std::distance(input, _scanState.next));

0 commit comments

Comments
 (0)