kyle/kte
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Terminal: draw control characters as ^X; bound GUI config values

Browse Source 
- C0 controls and DEL were passed to ncurses, which acted on them: a
  carriage return moved to column 0 and the line padding then erased
  the text, so every line of a CRLF file drew blank, and a backspace
  ate the previous character. They are now drawn in caret notation
  (^M, ^H, ^?) and counted as two cells. C1 controls (and stray bytes
  0x80-0x9F, which decode to them; U+009B is CSI) are drawn as '?'.
- The width rule lives in TermWidth.h and is shared by the renderer's
  drawing loop, its cursor placement and the command layer's
  horizontal scrolling and mouse mapping, which also disagreed on NUL
  and invalid bytes.
- GUI config: columns/rows must be 1..1000 and the font size a finite
  4..200 (TOML 'size = inf' and huge values were accepted, leading to
  UB in float-to-int conversion and int overflow downstream).

Verified in tmux: a CRLF file now shows 'first line^M' where the
baseline showed a blank line.

Audit findings B9 (and the CR/BS case from the follow-up review).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
This commit is contained in:
Claude
2026-09-24 09:16:04 -07:00
committed by Kyle Isom
parent eaa605f548
commit 7e16680a26
5 changed files with 109 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Command.cc
+9 -9
View File
@@ -12,6 +12,7 @@
#include "Command.h"
#include "RegexGuard.h"
#include "TermWidth.h"
#include "syntax/HighlighterRegistry.h"
#include "syntax/NullHighlighter.h"
#include "Editor.h"
@@ -134,17 +135,16 @@ measure_char(std::string_view line, std::size_t i, std::size_t rx, std::size_t t
wchar_t wch = 0;
const auto rc = std::mbrtowc(&wch, line.data() + i, line.size() - i, &state);
if (rc == static_cast<std::size_t>(-1) || rc == static_cast<std::size_t>(-2) || rc == 0) {
len = 1;
width = (rc == 0) ? 0 : 1;
return;
}
len = rc;
if (wch == L'\t') {
width = tabw - (rx % tabw);
// Invalid byte or NUL: the renderer draws the byte value itself.
wch = static_cast<unsigned char>(line[i]);
len = 1;
} else {
const int w = wcwidth(wch);
width = (w < 0) ? 1 : static_cast<std::size_t>(w);
len = rc;
}
if (wch == L'\t')
width = tabw - (rx % tabw);
else
width = static_cast<std::size_t>(kte::CellWidth(wch));
}
GUIConfig.cc
+24 -6
View File
@@ -1,3 +1,4 @@
#include <cmath>
#include <cstdlib>
#include <cstring>
#include <fstream>
@@ -20,6 +21,7 @@
#include "ext/tomlplusplus/toml.hpp"
#if defined(__clang__)
# pragma clang diagnostic pop
#elif defined(__GNUC__)
@@ -27,6 +29,22 @@
#endif
// Reject values that would overflow later arithmetic (columns * font size)
// or that are not usable: "size = inf" is valid TOML.
static bool
valid_dimension(long long v)
{
return v > 0 && v <= 1000;
}
static bool
valid_font_size(double v)
{
return std::isfinite(v) && v >= 4.0 && v <= 200.0;
}
static void
trim(std::string &s)
{
@@ -91,10 +109,10 @@ GUIConfig::LoadFromTOML(const std::string &path)
if (auto v = (*win)["fullscreen"].value<bool>())
fullscreen = *v;
if (auto v = (*win)["columns"].value<int64_t>()) {
if (*v > 0) columns = static_cast<int>(*v);
if (valid_dimension(*v)) columns = static_cast<int>(*v);
}
if (auto v = (*win)["rows"].value<int64_t>()) {
if (*v > 0) rows = static_cast<int>(*v);
if (valid_dimension(*v)) rows = static_cast<int>(*v);
}
}
@@ -105,7 +123,7 @@ GUIConfig::LoadFromTOML(const std::string &path)
if (auto v = (*sec)["name"].value<std::string>())
font = *v;
if (auto v = (*sec)["size"].value<double>()) {
if (*v > 0.0) font_size = static_cast<float>(*v);
if (valid_font_size(*v)) font_size = static_cast<float>(*v);
}
if (auto v = (*sec)["code"].value<std::string>()) {
code_font = *v;
@@ -192,21 +210,21 @@ GUIConfig::LoadFromINI(const std::string &path)
try {
v = std::stoi(val);
} catch (...) {}
if (v > 0)
if (valid_dimension(v))
columns = v;
} else if (key == "rows") {
int v = rows;
try {
v = std::stoi(val);
} catch (...) {}
if (v > 0)
if (valid_dimension(v))
rows = v;
} else if (key == "font_size" || key == "fontsize") {
float v = font_size;
try {
v = std::stof(val);
} catch (...) {}
if (v > 0.0f) {
if (valid_font_size(v)) {
font_size = v;
}
} else if (key == "font") {
TermWidth.h
+40
View File
@@ -0,0 +1,40 @@
// TermWidth.h - terminal cell width of a decoded character
//
// Shared by TerminalRenderer (drawing and cursor placement) and the command
// layer (horizontal scrolling, mouse column mapping) so they always agree.
// Tabs are handled by the callers, which know the current column.
#pragma once
#include <cwchar>
namespace kte {
// C0 controls and DEL are drawn as caret notation ("^M", "^?"): two cells.
// ncurses would otherwise act on them (carriage return, backspace) and
// corrupt the line.
inline bool
IsCaretControl(wchar_t wch)
{
return (wch >= 0 && wch < 0x20) || wch == 0x7f;
}
// C1 controls (U+0080..U+009F, also what a stray byte in that range decodes
// to) are drawn as '?': sent raw, U+009B is a terminal CSI.
inline bool
IsC1Control(wchar_t wch)
{
return wch >= 0x80 && wch <= 0x9f;
}
inline int
CellWidth(wchar_t wch)
{
if (IsCaretControl(wch))
return 2;
if (IsC1Control(wch))
return 1;
const int w = wcwidth(wch);
return w < 0 ? 1 : w;
}
} // namespace kte
TerminalRenderer.cc
+23 -21
View File
@@ -11,6 +11,7 @@
#include "TerminalRenderer.h"
#include "RegexGuard.h"
#include "TermWidth.h"
#include "Buffer.h"
#include "Editor.h"
#include "Highlight.h"
@@ -177,10 +178,7 @@ TerminalRenderer::Draw(Editor &ed)
const std::size_t next_tab = tab_width - (rc % tab_width);
rc += next_tab;
} else {
int w = wcwidth(wch);
if (w < 0)
w = 1;
rc += static_cast<std::size_t>(w);
rc += static_cast<std::size_t>(kte::CellWidth(wch));
}
si += static_cast<std::size_t>(wch_len);
}
@@ -315,10 +313,8 @@ TerminalRenderer::Draw(Editor &ed)
src_i += wch_len;
continue;
} else {
// normal char
disp_w = wcwidth(wch);
if (disp_w < 0)
disp_w = 1; // non-printable or similar
// normal char (controls are drawn as ^X or '?')
disp_w = kte::CellWidth(wch);
if (render_col < coloffs) {
render_col += disp_w;
@@ -366,7 +362,12 @@ TerminalRenderer::Draw(Editor &ed)
}
attrset(a);
if (from_src) {
if (from_src && kte::IsCaretControl(wch)) {
addch('^');
addch(wch == 0x7f ? '?' : static_cast<chtype>(wch + 0x40));
} else if (from_src && kte::IsC1Control(wch)) {
addch('?');
} else if (from_src) {
cchar_t cch;
wchar_t warr[2] = {wch, L'\0'};
setcchar(&cch, warr, 0, 0, nullptr);
@@ -405,21 +406,22 @@ TerminalRenderer::Draw(Editor &ed)
&wch, &line_for_cursor[src_i_cur], line_for_cursor.size() - src_i_cur,
&state);
// Same decoding and widths as the drawing loop above.
std::size_t len = res;
if (res == (size_t) -1 || res == (size_t) -2) {
render_col_cur += 1;
src_i_cur += 1;
wch = static_cast<unsigned char>(line_for_cursor[src_i_cur]);
len = 1;
} else if (res == 0) {
src_i_cur += 1;
} else {
if (wch == L'\t') {
std::size_t next_tab = tabw - (render_col_cur % tabw);
render_col_cur += next_tab;
} else {
int dw = wcwidth(wch);
render_col_cur += (dw < 0) ? 1 : dw;
}
src_i_cur += res;
wch = L'\0';
len = 1;
}
if (wch == L'\t') {
std::size_t next_tab = tabw - (render_col_cur % tabw);
render_col_cur += next_tab;
} else {
render_col_cur += static_cast<std::size_t>(kte::CellWidth(wch));
}
src_i_cur += len;
}
rx_recomputed = render_col_cur;
}
tests/test_audit_regressions.cc
+13
View File
@@ -818,3 +818,16 @@ TEST(Audit_SaveAs_ExistingFile_AsksFirst)
ASSERT_TRUE(h.Exec(CommandId::Newline));
ASSERT_EQ(slurp(d.path / "other.txt"), std::string("precious\n"));
}
// Control characters are drawn as ^X (two cells); horizontal scrolling must
// count them the same way.
TEST(Audit_HorizontalScroll_ControlCharsAreTwoCells)
{
TestHarness h;
Buffer &b = h.Buf();
b.insert_text(0, 0, std::string(50, '\r'));
b.SetCursor(0, 0);
ASSERT_TRUE(h.Exec(CommandId::MoveEnd));
ASSERT_EQ(b.Coloffs(), (std::size_t) (100 - 80 + 1));
}