Skip to content

Commit bf00f51

Browse files
committed
readline: Improve robustness of editing and escape-sequence handling
Replace fragile cursor compensation with full-line redraws where needed, fix Ctrl+W buffer/screen corruption, properly consume unrecognized CSI sequences, and always clean up leaked local-echo characters from buggy serial drivers. These changes make line editing consistent across terminals while preserving existing behavior. Signed-off-by: Alan C. Assis <acassis@gmail.com> Assisted-by: Claude Code (Sonnet 5)
1 parent 5779652 commit bf00f51

2 files changed

Lines changed: 134 additions & 48 deletions

File tree

system/readline/Kconfig

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,18 +61,15 @@ config READLINE_EDIT
6161
Build in support for full command-line editing using cursor keys,
6262
Home/End, Delete. Requires a VT100/ANSI-compatible terminal.
6363

64-
if READLINE_EDIT
65-
6664
config READLINE_EDIT_EMACS
6765
bool "Emacs-style control keys"
6866
default n
67+
depends on READLINE_EDIT
6968
---help---
7069
Enable emacs-style control key bindings:
7170
Ctrl+A Home, Ctrl+E End, Ctrl+K kill-to-end,
7271
Ctrl+U kill-to-start, Ctrl+Left/Right word movement.
7372

74-
endif # READLINE_EDIT
75-
7673
config READLINE_CMD_HISTORY
7774
bool "Command line history"
7875
default n

system/readline/readline_common.c

Lines changed: 133 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ static int word_skip(FAR const char *buf, int cursor, int bound,
543543
#endif
544544

545545
#ifdef CONFIG_READLINE_ECHO
546-
#ifdef CONFIG_READLINE_EDIT
546+
# ifdef CONFIG_READLINE_EDIT
547547
/****************************************************************************
548548
* Name: redraw_line
549549
*
@@ -621,9 +621,9 @@ static void redraw_tail(FAR struct rl_common_s *vtbl, FAR const char *buf,
621621
}
622622
}
623623
}
624-
#endif /* CONFIG_READLINE_EDIT */
624+
# endif /* CONFIG_READLINE_EDIT */
625625

626-
#ifdef CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH
626+
# ifdef CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH
627627
/****************************************************************************
628628
* Name: isearch_redraw
629629
*
@@ -669,7 +669,7 @@ static void isearch_redraw(FAR struct rl_common_s *vtbl,
669669
RL_WRITE(vtbl, buf, nch);
670670
}
671671
}
672-
#endif /* CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH */
672+
# endif /* CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH */
673673
#endif /* CONFIG_READLINE_ECHO */
674674

675675
/****************************************************************************
@@ -799,13 +799,13 @@ FAR const char *readline_prompt(FAR const char *prompt)
799799
FAR const struct extmatch_vtable_s *
800800
readline_extmatch(FAR const struct extmatch_vtable_s *vtbl)
801801
{
802-
#if (CONFIG_READLINE_MAX_EXTCMDS > 0)
802+
# if (CONFIG_READLINE_MAX_EXTCMDS > 0)
803803
FAR const struct extmatch_vtable_s *ret = g_extmatch_vtbl;
804804
g_extmatch_vtbl = vtbl;
805805
return ret;
806-
#else
806+
# else
807807
return NULL;
808-
#endif
808+
# endif
809809
}
810810
#endif
811811

@@ -934,9 +934,9 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
934934

935935
bool found = isearch_find(search, searchlen, searchoffset,
936936
&searchoffset, buf, buflen, &nch);
937-
#ifdef CONFIG_READLINE_ECHO
937+
# ifdef CONFIG_READLINE_ECHO
938938
isearch_redraw(vtbl, search, searchlen, buf, nch, !found);
939-
#endif
939+
# endif
940940
}
941941
else if (ch == ASCII_BS || ch == ASCII_DEL)
942942
{
@@ -956,10 +956,10 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
956956
}
957957
}
958958

959-
#ifdef CONFIG_READLINE_ECHO
959+
# ifdef CONFIG_READLINE_ECHO
960960
isearch_redraw(vtbl, search, searchlen, buf, nch,
961961
searchlen > 0 && nch == 0);
962-
#endif
962+
# endif
963963
}
964964
else if (ch == CTRL_G || ch == ASCII_ETX) /* ^G or ^C: cancel */
965965
{
@@ -978,9 +978,9 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
978978

979979
buf[nch] = '\0';
980980

981-
#ifdef CONFIG_READLINE_ECHO
981+
# ifdef CONFIG_READLINE_ECHO
982982
redraw_line(vtbl, buf, nch, cursor);
983-
#endif
983+
# endif
984984
}
985985
else if (ch == '\n')
986986
{
@@ -1009,9 +1009,9 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
10091009

10101010
bool found = isearch_find(search, searchlen, searchoffset,
10111011
&searchoffset, buf, buflen, &nch);
1012-
#ifdef CONFIG_READLINE_ECHO
1012+
# ifdef CONFIG_READLINE_ECHO
10131013
isearch_redraw(vtbl, search, searchlen, buf, nch, !found);
1014-
#endif
1014+
# endif
10151015
}
10161016
else
10171017
{
@@ -1023,14 +1023,14 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
10231023
insearch = false;
10241024
cursor = nch;
10251025

1026-
#ifdef CONFIG_READLINE_ECHO
1026+
# ifdef CONFIG_READLINE_ECHO
10271027
redraw_line(vtbl, buf, nch, cursor);
1028-
#endif
1028+
# endif
10291029
}
10301030

10311031
continue;
10321032
}
1033-
#endif
1033+
#endif /* CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH */
10341034

10351035
/* Are we processing a VT100 escape sequence */
10361036

@@ -1042,19 +1042,44 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
10421042
if (escape == 3)
10431043
{
10441044
escape = 0;
1045-
if (ch == '~' && cursor < nch)
1045+
if (ch == '~')
10461046
{
1047-
int k;
1048-
for (k = cursor + 1; k < nch; k++)
1049-
buf[k - 1] = buf[k];
1050-
nch--;
1047+
if (cursor < nch)
1048+
{
1049+
int k;
1050+
for (k = cursor + 1; k < nch; k++)
1051+
buf[k - 1] = buf[k];
1052+
nch--;
1053+
}
1054+
10511055
# ifdef CONFIG_READLINE_ECHO
1052-
/* Back up 1 — terminal echo of '~' advanced cursor */
1056+
/* Delete arrives as the 4-byte sequence "ESC [ 3 ~".
1057+
* Some serial drivers only know how to suppress
1058+
* local echo for fixed 3-byte "ESC [ x" sequences
1059+
* and leak the trailing '~' as a literal character,
1060+
* silently advancing the terminal's real cursor by
1061+
* one column. redraw_line() is immune to this (and
1062+
* to any other such drift) because it always
1063+
* returns to column 0 with '\r' first, rather than
1064+
* assuming where the cursor currently is -- unlike
1065+
* a plain erase-and-rewrite-the-tail redraw, which
1066+
* only works relative to the cursor's last known
1067+
* position and has no way to recover if that
1068+
* assumption turns out to be wrong.
1069+
*
1070+
* This redraw is unconditional -- even when there
1071+
* was nothing to delete (cursor already at the true
1072+
* end, including an empty line) -- specifically so
1073+
* it also cleans up a leaked '~' in that case; only
1074+
* skipping the redraw when there is "nothing to do"
1075+
* is exactly what left a stray '~' visible on
1076+
* screen with an unpatched driver.
1077+
*/
10531078

1054-
RL_WRITE(vtbl, g_curleft, sizeof(g_curleft));
1055-
redraw_tail(vtbl, buf, nch, cursor);
1079+
redraw_line(vtbl, buf, nch, cursor);
10561080
# endif
10571081
}
1082+
10581083
continue;
10591084
}
10601085

@@ -1117,6 +1142,35 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
11171142
}
11181143
#endif /* CONFIG_READLINE_EDIT */
11191144

1145+
if (escape == 10) /* Unrecognized CSI -- consume to final byte */
1146+
{
1147+
if (ch >= 0x20 && ch <= 0x3f)
1148+
{
1149+
continue; /* still a parameter/intermediate byte */
1150+
}
1151+
1152+
/* ch is the final byte (0x40-0x7e), or something
1153+
* malformed -- either way the sequence is over now and
1154+
* is discarded; nothing was ever inserted into the
1155+
* buffer. Still issue a corrective redraw: some serial
1156+
* drivers only know how to suppress local echo for
1157+
* fixed 3-byte "ESC [ x" sequences and leak bytes from
1158+
* any longer, unrecognized sequence as literal
1159+
* characters, the same class of issue Delete has for
1160+
* its own "ESC [ 3 ~". redraw_line() cleans up any
1161+
* such leaked characters even though the buffer itself
1162+
* was never affected by them.
1163+
*/
1164+
1165+
escape = 0;
1166+
# ifdef CONFIG_READLINE_ECHO
1167+
# ifdef CONFIG_READLINE_EDIT
1168+
redraw_line(vtbl, buf, nch, cursor);
1169+
# endif
1170+
# endif
1171+
continue;
1172+
}
1173+
11201174
/* Some terminals (e.g. xterm) use the SS3 introducer "ESC O"
11211175
* rather than CSI "ESC [" for Home/End (and, in application
11221176
* cursor key mode, for the arrow keys too). Recognize the
@@ -1196,10 +1250,10 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
11961250

11971251
nch = 0;
11981252

1199-
#ifdef CONFIG_READLINE_ECHO
1253+
# ifdef CONFIG_READLINE_ECHO
12001254
RL_PUTC(vtbl, '\r');
12011255
RL_WRITE(vtbl, g_erasetoeol, sizeof(g_erasetoeol));
1202-
#if defined(CONFIG_READLINE_TABCOMPLETION) || defined(CONFIG_READLINE_EDIT)
1256+
# if defined(CONFIG_READLINE_TABCOMPLETION) || defined(CONFIG_READLINE_EDIT)
12031257
if (g_readline_prompt != NULL)
12041258
{
12051259
int k;
@@ -1208,8 +1262,8 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
12081262
RL_PUTC(vtbl, g_readline_prompt[k]);
12091263
}
12101264
}
1211-
#endif
1212-
#endif
1265+
# endif
1266+
# endif
12131267

12141268
if (g_cmdhist.offset != 1)
12151269
{
@@ -1244,9 +1298,9 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
12441298
* using a stale, out-of-range cursor value.
12451299
*/
12461300

1247-
#ifdef CONFIG_READLINE_EDIT
1301+
# ifdef CONFIG_READLINE_EDIT
12481302
cursor = nch;
1249-
#endif
1303+
# endif
12501304
}
12511305
#endif /* CONFIG_READLINE_CMD_HISTORY */
12521306

@@ -1315,6 +1369,22 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
13151369
}
13161370
#endif
13171371

1372+
/* Not one of the specific sequences recognized above. If
1373+
* this still looks like a CSI parameter or intermediate
1374+
* byte (0x20-0x3f: digits, ';', and similar), this is
1375+
* some other CSI sequence this code doesn't specifically
1376+
* know about -- keep consuming bytes until the real final
1377+
* byte (0x40-0x7e) ends it, rather than treating this
1378+
* byte as the end of the sequence and leaking whatever
1379+
* comes after it as literal input.
1380+
*/
1381+
1382+
if (ch >= 0x20 && ch <= 0x3f)
1383+
{
1384+
escape = 10;
1385+
continue;
1386+
}
1387+
13181388
escape = 0;
13191389
ch = 'a';
13201390
}
@@ -1450,19 +1520,34 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
14501520
buf[nch] = '\0';
14511521
redraw_line(vtbl, buf, nch, cursor);
14521522
}
1453-
else if (ch == CTRL_W) /* Ctrl+W = Kill word backward (FIXME) */
1523+
else if (ch == CTRL_W) /* Ctrl+W = Kill word backward */
14541524
{
14551525
int start, k;
14561526
start = word_skip(buf, cursor, 0, false);
1457-
for (k = start; k < nch; k++)
1527+
for (k = cursor; k < nch; k++)
14581528
buf[k - (cursor - start)] = buf[k];
14591529
nch -= (cursor - start);
14601530
cursor = start;
14611531
buf[nch] = '\0';
1462-
redraw_tail(vtbl, buf, nch, cursor);
1532+
1533+
/* Unlike Ctrl+K (which never moves the cursor, so erasing
1534+
* from wherever it already is is correct) and Backspace
1535+
* (which moves it by exactly one column and compensates
1536+
* with a single explicit backspace byte), Ctrl+W can move
1537+
* the cursor back by any number of columns depending on how
1538+
* long the killed word was. redraw_tail() has no way to
1539+
* express "also move left by N first"; it just erases from
1540+
* wherever the terminal's cursor already happens to be.
1541+
* Using redraw_line() instead sidesteps the whole problem
1542+
* the same way it does for Delete: it returns to column 0
1543+
* with '\r' first, so it is correct regardless of how far
1544+
* the cursor moved.
1545+
*/
1546+
1547+
redraw_line(vtbl, buf, nch, cursor);
14631548
}
14641549

1465-
#ifdef CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH
1550+
# ifdef CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH
14661551
else if (ch == CTRL_R && g_cmdhist.len > 0)
14671552
{
14681553
/* Ctrl+R = start a reverse incremental search through the
@@ -1489,11 +1574,11 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
14891574
savedcursor = cursor;
14901575
nch = 0;
14911576

1492-
#ifdef CONFIG_READLINE_ECHO
1577+
# ifdef CONFIG_READLINE_ECHO
14931578
isearch_redraw(vtbl, search, searchlen, buf, nch, false);
1494-
#endif
1579+
# endif
14951580
}
1496-
#endif /* CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH */
1581+
# endif /* CONFIG_READLINE_EDIT_EMACS_REVERSE_SEARCH */
14971582
#endif /* CONFIG_READLINE_EDIT_EMACS */
14981583

14991584
/* Otherwise, put the character in the line buffer if the
@@ -1526,15 +1611,19 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
15261611
if (nch + 1 < buflen)
15271612
{
15281613
int j;
1529-
for (j = nch; j > cursor; j--) buf[j] = buf[j - 1];
1614+
for (j = nch; j > cursor; j--)
1615+
{
1616+
buf[j] = buf[j - 1];
1617+
}
1618+
15301619
buf[cursor] = (char)ch; nch++; cursor++;
15311620
# ifdef CONFIG_READLINE_ECHO
15321621
redraw_tail(vtbl, buf, nch, cursor);
15331622
# endif
15341623
}
15351624
#else
15361625
buf[nch++] = ch;
1537-
#endif
1626+
#endif /* CONFIG_READLINE_EDIT */
15381627

15391628
/* Check if there is room for another character and the line's
15401629
* null terminator. If not then we have to end the line now.
@@ -1569,14 +1658,14 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
15691658
* only in the opposite direction.
15701659
*/
15711660

1572-
#ifdef CONFIG_READLINE_EDIT
1661+
# ifdef CONFIG_READLINE_EDIT
15731662
if (tab_completion(vtbl, buf, buflen, &nch))
15741663
{
15751664
cursor = nch;
15761665
}
1577-
#else
1666+
# else
15781667
tab_completion(vtbl, buf, buflen, &nch);
1579-
#endif
1668+
# endif
15801669
}
15811670
#endif
15821671
}

0 commit comments

Comments
 (0)