Просмотр исходного кода

fix(ui): consistent frame glyphs on Windows — agree with clack, keep raw path ASCII (#1307)

codegraph's glyphs were ASCII on every Windows console while
@clack/prompts drew its Unicode frame around them, so one index block
mixed `|` and `│` rails (#398). supportsUnicode() now mirrors the
is-unicode-supported detection clack bundles (Windows Terminal, VS
Code, ConEmu/Cmder, Alacritty, xterm-256color, JetBrains, CI), so both
systems always pick the same glyph family.

The shimmer worker's raw fs.writeSync(1) bytes still decode through the
console codepage (OEM codepages mojibake UTF-8 even under Windows
Terminal — the #168 regression to avoid), so:

- the raw path gets its own supportsUnicodeRawWrites() that stays ASCII
  on win32 unless CODEGRAPH_UNICODE=1, and
- the persistent "phase done" lines move from the worker to the parent,
  written via process.stdout (wide-char console API, codepage-immune) at
  phase transitions — the main thread is alive there, it's delivering
  the progress callback. Only transient, self-erasing animation frames
  remain on the raw path, so ASCII never lands in scrollback.

Fixes #398

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Colby Mchenry 1 месяц назад
Родитель
Сommit
30421953ac
6 измененных файлов с 209 добавлено и 32 удалено
  1. 1 0
      CHANGELOG.md
  2. 88 4
      __tests__/glyphs.test.ts
  3. 58 11
      src/ui/glyphs.ts
  4. 50 4
      src/ui/shimmer-progress.ts
  5. 12 12
      src/ui/shimmer-worker.ts
  6. 0 1
      src/ui/types.ts

+ 1 - 0
CHANGELOG.md

@@ -18,6 +18,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- Progress output on Windows no longer mixes ASCII `|` rails with the Unicode `│ ◆ ●` frame around them. In terminals that render Unicode (Windows Terminal, VS Code, ConEmu/Cmder, JetBrains, Alacritty), the whole `codegraph init` / `index` / `sync` block now draws with matching box-drawing characters; unrecognized legacy consoles keep the safe all-ASCII output that avoids garbled characters. `CODEGRAPH_ASCII=1` / `CODEGRAPH_UNICODE=1` still override in either direction. (#398)
 - CLI output now honors the `NO_COLOR` convention and new `--color` / `--no-color` flags, and goes plain automatically when piped: commands like `codegraph status`, `query`, `callers`, and `files` no longer embed ANSI color codes when stdout isn't a terminal, and a piped `codegraph init` / `index` / `sync` prints simple per-phase lines instead of progress-animation control characters. `FORCE_COLOR` or `--color` forces color back on for pipes that render it. (#1281)
 - Callers and impact analysis no longer silently under-count a function that calls the same callee many times. When one caller contained several call sites to the same callee and an internal resolution batch boundary happened to split them, cleanup after the first batch removed the later sites' pending rows before they were ever attempted — their edges were never created, deterministically, and which edges went missing shifted with unrelated changes to the project's total reference count. Post-pass cleanup now targets the exact database row each processed reference came from. Found while validating the operator-call fix on nlohmann/json, where `write_cbor`'s 11 calls to `to_char_type` indexed as 10. (#1269)
 - C++ explicit operator calls — `a.operator+(b)`, `p->operator+(b)`, `a.operator[](3)`, and the other symbolic forms — now produce a `calls` edge to the operator method, so an operator invoked only through the explicit syntax no longer looks uncalled in callers and impact analysis. tree-sitter parses these call sites with the operator name stranded in an error node (never as a normal member access), so the call's target was silently read as just the receiver variable; the operator name is now recovered from the error node and resolved through receiver-type inference like any other member call — a same-named operator on an unrelated class can never capture the edge. Infix uses (`a + b`, `a[i]`) need real type inference and are tracked separately. (#1247)

+ 88 - 4
__tests__/glyphs.test.ts

@@ -10,6 +10,7 @@
 import { describe, it, expect, beforeEach, afterEach } from 'vitest';
 import {
   supportsUnicode,
+  supportsUnicodeRawWrites,
   getGlyphs,
   UNICODE_GLYPHS,
   ASCII_GLYPHS,
@@ -54,13 +55,84 @@ describe('supportsUnicode', () => {
     _resetGlyphsCache();
   });
 
-  it('returns false on Windows by default (mojibake-prone consoles)', () => {
-    withEnv({ CODEGRAPH_ASCII: undefined, CODEGRAPH_UNICODE: undefined, TERM: undefined }, () => {
+  /** Clears every signal the Windows detection reads, so cases are explicit. */
+  const NO_TERMINAL_SIGNALS: Record<string, string | undefined> = {
+    CODEGRAPH_ASCII: undefined,
+    CODEGRAPH_UNICODE: undefined,
+    TERM: undefined,
+    CI: undefined,
+    WT_SESSION: undefined,
+    TERMINUS_SUBLIME: undefined,
+    ConEmuTask: undefined,
+    TERM_PROGRAM: undefined,
+    TERMINAL_EMULATOR: undefined,
+  };
+
+  it('returns false on Windows in an unrecognized console (mojibake-prone legacy conhost)', () => {
+    withEnv(NO_TERMINAL_SIGNALS, () => {
+      setPlatform('win32');
+      expect(supportsUnicode()).toBe(false);
+    });
+  });
+
+  // The Windows allowlist must match @clack/prompts' bundled detection —
+  // wherever clack draws its Unicode frame, our rails must be Unicode too,
+  // or `codegraph index` mixes `|` and `│` in one output block (#398).
+  it.each([
+    ['Windows Terminal', { WT_SESSION: 'a-guid' }],
+    ['VS Code terminal', { TERM_PROGRAM: 'vscode' }],
+    ['ConEmu/Cmder', { ConEmuTask: '{cmd::Cmder}' }],
+    ['Alacritty', { TERM: 'alacritty' }],
+    ['xterm-256color', { TERM: 'xterm-256color' }],
+    ['JetBrains terminal', { TERMINAL_EMULATOR: 'JetBrains-JediTerm' }],
+    ['CI', { CI: 'true' }],
+  ])('returns true on Windows in %s (agrees with clack, #398)', (_name, envPatch) => {
+    withEnv({ ...NO_TERMINAL_SIGNALS, ...envPatch }, () => {
+      setPlatform('win32');
+      expect(supportsUnicode()).toBe(true);
+    });
+  });
+
+  it('CODEGRAPH_ASCII=1 still wins inside Windows Terminal (escape hatch)', () => {
+    withEnv({ ...NO_TERMINAL_SIGNALS, CODEGRAPH_ASCII: '1', WT_SESSION: 'a-guid' }, () => {
       setPlatform('win32');
       expect(supportsUnicode()).toBe(false);
     });
   });
 
+  // The raw fs.writeSync(1) path (shimmer animation frames) decodes through
+  // the console CODEPAGE on Windows, so it must stay ASCII there even in
+  // terminals where the codepage-immune main-thread path goes Unicode (#168).
+  describe('supportsUnicodeRawWrites', () => {
+    it('stays ASCII on Windows even inside Windows Terminal / vscode / CI', () => {
+      for (const envPatch of [{ WT_SESSION: 'a-guid' }, { TERM_PROGRAM: 'vscode' }, { CI: 'true' }]) {
+        withEnv({ ...NO_TERMINAL_SIGNALS, ...envPatch }, () => {
+          setPlatform('win32');
+          expect(supportsUnicodeRawWrites()).toBe(false);
+          expect(supportsUnicode()).toBe(true); // main-thread path DOES go Unicode there
+        });
+      }
+    });
+
+    it('CODEGRAPH_UNICODE=1 opts the raw path in on Windows', () => {
+      withEnv({ ...NO_TERMINAL_SIGNALS, CODEGRAPH_UNICODE: '1' }, () => {
+        setPlatform('win32');
+        expect(supportsUnicodeRawWrites()).toBe(true);
+      });
+    });
+
+    it('matches supportsUnicode() off Windows (Unicode on macOS, ASCII on TERM=linux)', () => {
+      withEnv({ ...NO_TERMINAL_SIGNALS }, () => {
+        setPlatform('darwin');
+        expect(supportsUnicodeRawWrites()).toBe(true);
+      });
+      withEnv({ ...NO_TERMINAL_SIGNALS, TERM: 'linux' }, () => {
+        setPlatform('linux');
+        expect(supportsUnicodeRawWrites()).toBe(false);
+      });
+    });
+  });
+
   it('returns true on macOS by default', () => {
     withEnv({ CODEGRAPH_ASCII: undefined, CODEGRAPH_UNICODE: undefined, TERM: undefined }, () => {
       setPlatform('darwin');
@@ -117,8 +189,20 @@ describe('getGlyphs', () => {
     _resetGlyphsCache();
   });
 
-  it('returns ASCII glyphs on Windows', () => {
-    withEnv({ CODEGRAPH_ASCII: undefined, CODEGRAPH_UNICODE: undefined }, () => {
+  it('returns ASCII glyphs on Windows in an unrecognized console', () => {
+    withEnv(
+      {
+        CODEGRAPH_ASCII: undefined,
+        CODEGRAPH_UNICODE: undefined,
+        TERM: undefined,
+        CI: undefined,
+        WT_SESSION: undefined,
+        TERMINUS_SUBLIME: undefined,
+        ConEmuTask: undefined,
+        TERM_PROGRAM: undefined,
+        TERMINAL_EMULATOR: undefined,
+      },
+      () => {
       setPlatform('win32');
       const g = getGlyphs();
       expect(g).toBe(ASCII_GLYPHS);

+ 58 - 11
src/ui/glyphs.ts

@@ -2,18 +2,30 @@
  * Glyph selection for CLI output.
  *
  * On Windows, console output is interpreted via the active output
- * codepage. PowerShell 5.1 and cmd.exe default to OEM codepages
- * (CP437, CP936, ...), so UTF-8 bytes written to the console render
- * as mojibake (see #168). The shimmer worker is hit hardest because
- * it uses `fs.writeSync(1, ...)` (raw bytes, no TTY-aware encoding
- * conversion) to keep animation smooth while the main thread is
- * blocked in SQLite. To stay readable everywhere, we fall back to
- * ASCII glyphs whenever the terminal is not known to handle UTF-8.
+ * codepage. PowerShell 5.1 and cmd.exe in legacy conhost default to
+ * OEM codepages (CP437, CP936, ...), so UTF-8 bytes written to the
+ * console render as mojibake (see #168). The shimmer worker is hit
+ * hardest because it uses `fs.writeSync(1, ...)` (raw bytes, no
+ * TTY-aware encoding conversion) to keep animation smooth while the
+ * main thread is blocked in SQLite. To stay readable everywhere, we
+ * fall back to ASCII glyphs whenever the terminal is not known to
+ * handle UTF-8.
  *
- * Detection is intentionally simple:
+ * The Windows branch must agree with @clack/prompts (which bundles
+ * `is-unicode-supported`): clack draws the outer `┌ │ └` frame around
+ * init/index/sync output, and if it decides Unicode while we decide
+ * ASCII, one block mixes `│` and `|` rails (#398). The terminals the
+ * list recognizes (Windows Terminal, VS Code, ConEmu/Cmder, Alacritty,
+ * JetBrains, Terminus, CI log viewers) all run with a UTF-8-capable
+ * output path, so the raw-byte shimmer writes render correctly there
+ * too; unrecognized Windows consoles keep the safe ASCII fallback —
+ * and clack falls back to ASCII in those as well, so output stays
+ * consistent in both directions.
+ *
+ * Detection:
  *   - `CODEGRAPH_ASCII=1`  -> ASCII (escape hatch for any terminal)
- *   - `CODEGRAPH_UNICODE=1` -> Unicode (opt-in on Windows)
- *   - Windows              -> ASCII by default
+ *   - `CODEGRAPH_UNICODE=1` -> Unicode (opt-in on any terminal)
+ *   - Windows              -> mirror is-unicode-supported (see above)
  *   - Linux kernel console (`TERM=linux`) -> ASCII
  *   - Everything else      -> Unicode
  */
@@ -21,7 +33,20 @@
 export function supportsUnicode(): boolean {
   if (process.env.CODEGRAPH_ASCII === '1') return false;
   if (process.env.CODEGRAPH_UNICODE === '1') return true;
-  if (process.platform === 'win32') return false;
+  if (process.platform === 'win32') {
+    const env = process.env;
+    return Boolean(
+      env.CI ||
+        env.WT_SESSION || // Windows Terminal
+        env.TERMINUS_SUBLIME ||
+        env.ConEmuTask === '{cmd::Cmder}' || // ConEmu and cmder
+        env.TERM_PROGRAM === 'Terminus-Sublime' ||
+        env.TERM_PROGRAM === 'vscode' ||
+        env.TERM === 'xterm-256color' ||
+        env.TERM === 'alacritty' ||
+        env.TERMINAL_EMULATOR === 'JetBrains-JediTerm'
+    );
+  }
   return process.env.TERM !== 'linux';
 }
 
@@ -85,6 +110,28 @@ export function getGlyphs(): Glyphs {
   return cached;
 }
 
+/**
+ * Unicode support for the RAW console write path — `fs.writeSync(1, ...)`,
+ * used only by the shimmer worker's transient animation frames. Raw bytes
+ * bypass Node's TTY-aware conversion and get decoded by the ACTIVE CONSOLE
+ * CODEPAGE on Windows; OEM codepages (CP437, CP936, ...) mojibake UTF-8
+ * there even inside Windows Terminal, whose ConPTY still decodes app output
+ * with the session codepage (#168). So the raw path stays ASCII on every
+ * Windows terminal unless the user opts in via CODEGRAPH_UNICODE=1 —
+ * independent of `supportsUnicode()`, which governs the codepage-immune
+ * main-thread writes (`process.stdout` uses the wide-char console API).
+ */
+export function supportsUnicodeRawWrites(): boolean {
+  if (process.env.CODEGRAPH_ASCII === '1') return false;
+  if (process.env.CODEGRAPH_UNICODE === '1') return true;
+  if (process.platform === 'win32') return false;
+  return process.env.TERM !== 'linux';
+}
+
+export function getRawWriteGlyphs(): Glyphs {
+  return supportsUnicodeRawWrites() ? UNICODE_GLYPHS : ASCII_GLYPHS;
+}
+
 /** Reset the cached glyph set. Test-only; production code should call `getGlyphs()`. */
 export function _resetGlyphsCache(): void {
   cached = null;

+ 50 - 4
src/ui/shimmer-progress.ts

@@ -1,6 +1,7 @@
 import { Worker } from 'worker_threads';
 import * as path from 'path';
 import { ansiColorsEnabled } from './color';
+import { getGlyphs } from './glyphs';
 
 const PHASE_NAMES: Record<string, string> = {
   scanning: 'Scanning files',
@@ -28,13 +29,45 @@ export function createShimmerProgress(): ShimmerProgress {
     return createPlainProgress();
   }
 
+  const useColor = ansiColorsEnabled();
+  const G = getGlyphs();
+  const DM = useColor ? '\x1b[2m' : '';
+  const GRN = useColor ? '\x1b[32m' : '';
+  const RST = useColor ? '\x1b[0m' : '';
+
   let lastPhase = '';
+  let lastPhaseName = '';
+  let lastPercent = -1;
+  let lastCount = 0;
+
+  // The persistent "phase done" lines — the ones that stay in scrollback —
+  // are printed HERE, on the main thread, not by the worker. process.stdout
+  // reaches a Windows console through the wide-char API, so these lines can
+  // carry the same Unicode glyphs @clack/prompts draws around them (#398);
+  // the worker's raw fs.writeSync path can't (codepage mojibake, #168) and is
+  // now used only for the transient, self-erasing animation frames. The main
+  // thread is guaranteed alive here: phase changes arrive via its own
+  // progress callback.
+  const printPhaseDone = (): void => {
+    if (!lastPhaseName) return;
+    let detail = '';
+    if (lastPercent >= 0) detail = ` ${G.dash} done`;
+    else if (lastCount > 0) detail = ` ${G.dash} ${lastCount.toLocaleString()} found`;
+    // Leading \r + erase clears the worker's in-flight animation line; one
+    // atomic write so a worker frame can't interleave mid-line.
+    process.stdout.write(
+      `\r\x1b[K${DM}${G.rail}${RST}  ${GRN}${G.phaseDone}${RST} ${lastPhaseName}${detail}\n`
+    );
+    lastPhaseName = '';
+    lastPercent = -1;
+    lastCount = 0;
+  };
 
   const workerPath = path.join(__dirname, 'shimmer-worker.js');
   const worker = new Worker(workerPath, {
     // colors:false keeps the animation (still an interactive TTY) but drops
     // the ANSI color codes, honoring NO_COLOR / --no-color (#1281).
-    workerData: { startTime: Date.now(), colors: ansiColorsEnabled() },
+    workerData: { startTime: Date.now(), colors: useColor },
   });
 
   return {
@@ -42,9 +75,10 @@ export function createShimmerProgress(): ShimmerProgress {
       const phaseName = PHASE_NAMES[progress.phase] || progress.phase;
 
       if (progress.phase !== lastPhase && lastPhase) {
-        worker.postMessage({ type: 'finish-phase' });
+        printPhaseDone();
       }
       lastPhase = progress.phase;
+      lastPhaseName = phaseName;
 
       let percent = -1;
       let count = 0;
@@ -53,6 +87,8 @@ export function createShimmerProgress(): ShimmerProgress {
       } else if (progress.current > 0) {
         count = progress.current;
       }
+      lastPercent = percent;
+      lastCount = count;
 
       worker.postMessage({
         type: 'update',
@@ -65,14 +101,24 @@ export function createShimmerProgress(): ShimmerProgress {
 
     stop() {
       return new Promise<void>((resolve) => {
+        let settled = false;
+        const finish = (): void => {
+          if (settled) return;
+          settled = true;
+          // Worker has cleared (or been terminated off) the animation line;
+          // persist the final phase's done-line from the main thread.
+          printPhaseDone();
+          resolve();
+        };
+
         const timeout = setTimeout(() => {
-          worker.terminate().then(() => resolve());
+          worker.terminate().then(finish);
         }, 2000);
 
         worker.on('message', (msg: { type: string }) => {
           if (msg.type === 'stopped') {
             clearTimeout(timeout);
-            worker.terminate().then(() => resolve());
+            worker.terminate().then(finish);
           }
         });
 

+ 12 - 12
src/ui/shimmer-worker.ts

@@ -1,6 +1,6 @@
 import { parentPort, workerData } from 'worker_threads';
 import { writeSync } from 'fs';
-import { getGlyphs } from './glyphs';
+import { getRawWriteGlyphs } from './glyphs';
 import type { ShimmerWorkerMessage } from './types';
 
 // Write directly to fd 1 (stdout) instead of writeStdout().
@@ -11,12 +11,16 @@ import type { ShimmerWorkerMessage } from './types';
 //
 // Side effect: bypasses Node's TTY-aware encoding conversion on Windows,
 // so UTF-8 bytes hit the console raw and mojibake on OEM codepages.
-// `getGlyphs()` returns ASCII fallbacks on Windows to avoid this (#168).
+// `getRawWriteGlyphs()` therefore always falls back to ASCII on Windows
+// (#168). Everything this worker writes is transient — erased by the next
+// frame or by the parent's phase-done line — so ASCII here never shows up
+// in scrollback (#398); the persistent lines are printed by the parent
+// through the codepage-immune process.stdout path.
 function writeStdout(s: string): void {
   writeSync(1, s);
 }
 
-const G = getGlyphs();
+const G = getRawWriteGlyphs();
 const SPINNER_GLYPHS = G.spinner;
 const ANIM_INTERVAL = 150;
 const FRAMES_PER_GLYPH = 3;
@@ -29,7 +33,6 @@ const COLORS: boolean = workerData.colors !== false;
 
 const RST = COLORS ? '\x1b[0m' : '';
 const DM = COLORS ? '\x1b[2m' : '';
-const GRN = COLORS ? '\x1b[32m' : '';
 const BOLD = COLORS ? '\x1b[1m' : '';
 
 const startTime: number = workerData.startTime;
@@ -104,13 +107,12 @@ function render(): void {
   writeStdout(`\r\x1b[K${line}`);
 }
 
-function finishPhase(): void {
+// Clear the in-flight animation line. The persistent "phase done" line is
+// printed by the PARENT on the main thread (TTY-aware, codepage-immune) —
+// this worker's raw-byte path must never leave bytes in scrollback (#398).
+function clearLine(): void {
   if (!currentMessage) return;
   writeStdout(`\r\x1b[K`);
-  let detail = '';
-  if (currentPercent >= 0) detail = ` ${G.dash} done`;
-  else if (currentCount > 0) detail = ` ${G.dash} ${formatNumber(currentCount)} found`;
-  writeStdout(`${DM}${G.rail}${RST}  ${GRN}${G.phaseDone}${RST} ${currentMessage}${detail}\n`);
   currentMessage = '';
   currentPercent = -1;
   currentCount = 0;
@@ -124,11 +126,9 @@ parentPort!.on('message', (msg: ShimmerWorkerMessage) => {
     currentMessage = msg.phaseName;
     currentPercent = msg.percent;
     currentCount = msg.count;
-  } else if (msg.type === 'finish-phase') {
-    finishPhase();
   } else if (msg.type === 'stop') {
     clearInterval(tickInterval);
-    finishPhase();
+    clearLine();
     parentPort!.postMessage({ type: 'stopped' });
   }
 });

+ 0 - 1
src/ui/types.ts

@@ -1,7 +1,6 @@
 /** Messages from main thread to worker */
 export type ShimmerWorkerMessage =
   | { type: 'update'; phase: string; phaseName: string; percent: number; count: number }
-  | { type: 'finish-phase' }
   | { type: 'stop' };
 
 /** Messages from worker to main thread */