fix(terminal): fallback to ASCII borders on legacy Windows consoles#41015
fix(terminal): fallback to ASCII borders on legacy Windows consoles#41015X-AlexBin wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR adds a Key findings:
Confidence Score: 3/5
Last reviewed commit: 3fbfcaa |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const isModernTerminal = | ||
| !!env.WT_SESSION || | ||
| term.includes("xterm") || | ||
| term.includes("cygwin") || | ||
| term.includes("msys") || | ||
| termProgram === "vscode"; | ||
|
|
||
| return isModernTerminal ? "unicode" : "ascii"; |
There was a problem hiding this comment.
Missing COLORTERM and ConEmu heuristics
The detection currently covers Windows Terminal (WT_SESSION), xterm-like shells, Cygwin, MSYS2, and VS Code — but misses a couple of widely-used Windows terminals that support Unicode:
- ConEmu / Cmder sets
ConEmuANSI=ON(orCMDER_ROOT) but does not setWT_SESSIONor aTERMvalue that contains "xterm". Without this check, ConEmu users would silently fall back to ASCII. COLORTERM=truecolor(or24bit) is a well-established signal for full-colour / modern terminal support and is set by many terminals that do support Unicode box-drawing characters.
Neither omission causes a crash, and the fallback to ASCII is safe. However, it is worth documenting the intentional scope limitation (or extending the checks) so that ConEmu users aren't caught off guard.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/terminal/table.ts
Line: 34-41
Comment:
**Missing `COLORTERM` and ConEmu heuristics**
The detection currently covers Windows Terminal (`WT_SESSION`), xterm-like shells, Cygwin, MSYS2, and VS Code — but misses a couple of widely-used Windows terminals that support Unicode:
- **ConEmu / Cmder** sets `ConEmuANSI=ON` (or `CMDER_ROOT`) but does not set `WT_SESSION` or a `TERM` value that contains "xterm". Without this check, ConEmu users would silently fall back to ASCII.
- **`COLORTERM=truecolor`** (or `24bit`) is a well-established signal for full-colour / modern terminal support and is set by many terminals that do support Unicode box-drawing characters.
Neither omission causes a crash, and the fallback to ASCII is safe. However, it is worth documenting the intentional scope limitation (or extending the checks) so that ConEmu users aren't caught off guard.
How can I resolve this? If you propose a fix, please make it concise.| return next; | ||
| }); | ||
| const border = opts.border ?? "unicode"; | ||
| const border = opts.border ?? resolveDefaultBorder(process.platform, process.env); |
There was a problem hiding this comment.
Existing tests will fail on Windows without a modern terminal
Several tests in src/terminal/table.test.ts assert against the Unicode vertical-bar character │ (U+2502) without explicitly setting border: "unicode":
// table.test.ts:18
expect(out).toMatch(/│ Dashboard\s+│/);
// table.test.ts:80-81
const lastSep = line.lastIndexOf("│");
expect(lastSep).toBeGreaterThan(resetIndex);
Before this PR, border always defaulted to "unicode", so those assertions were always safe. After this change, the default is determined at runtime by resolveDefaultBorder(process.platform, process.env). On a Windows machine where none of WT_SESSION, xterm/cygwin/msys in TERM, or TERM_PROGRAM=vscode is set, the function returns "ascii" and every test that hardcodes │ will fail.
The simplest fix is to pin border: "unicode" in the tests that care about the specific character used:
// table.test.ts – prefers shrinking flex columns
const out = renderTable({
border: "unicode",
// ...
});Doing this for the four affected tests (prefers shrinking flex columns, expands flex columns, wraps ANSI-colored cells, resets ANSI styling) ensures they remain deterministic regardless of the host environment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/terminal/table.ts
Line: 263
Comment:
**Existing tests will fail on Windows without a modern terminal**
Several tests in `src/terminal/table.test.ts` assert against the Unicode vertical-bar character `│` (U+2502) without explicitly setting `border: "unicode"`:
```
// table.test.ts:18
expect(out).toMatch(/│ Dashboard\s+│/);
// table.test.ts:80-81
const lastSep = line.lastIndexOf("│");
expect(lastSep).toBeGreaterThan(resetIndex);
```
Before this PR, `border` always defaulted to `"unicode"`, so those assertions were always safe. After this change, the default is determined at runtime by `resolveDefaultBorder(process.platform, process.env)`. On a Windows machine where none of `WT_SESSION`, `xterm`/`cygwin`/`msys` in `TERM`, or `TERM_PROGRAM=vscode` is set, the function returns `"ascii"` and every test that hardcodes `│` will fail.
The simplest fix is to pin `border: "unicode"` in the tests that care about the specific character used:
```ts
// table.test.ts – prefers shrinking flex columns
const out = renderTable({
border: "unicode",
// ...
});
```
Doing this for the four affected tests (`prefers shrinking flex columns`, `expands flex columns`, `wraps ANSI-colored cells`, `resets ANSI styling`) ensures they remain deterministic regardless of the host environment.
How can I resolve this? If you propose a fix, please make it concise.
|
Should it also check the current code page? |
|
Canonical follow-up is now #43520. I kept that PR focused to the legacy-Windows table-border fallback and added explicit regression coverage for legacy vs. modern Windows terminal defaults. |
|
Fix #43520 |
4 participants
Summary
Fixes garbled Unicode table borders for commands like
openclaw skills liston Windows PowerShell/CMD with default GBK (936) code page by automatically falling back to ASCII borders on legacy consoles.Change Type
Scope
Linked Issue
Details
resolveDefaultBorder(platform, env)insrc/terminal/table.tsto pick a safe default border style.win32, use heuristics to detect modern terminals (Windows Terminal, VS Code, xterm-like) and keep Unicode there, otherwise default to ASCII (+-|) to avoid mojibake on GBK consoles.opts.borderfully honored so callers can still explicitly choose"unicode" | "ascii" | "none".