Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/core/cli/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,29 @@ export function paintChunk(text, atLineStart) {
return parts.join('\n')
}

/** The hook a colorized wrap answers to, keyed so nothing reaches it by guessing. */
const RESYNC = Symbol('colorizeStderr.resyncLineStart')

/**
* Tell a colorized stream the terminal is back at a line start.
*
* {@link colorizeStderr} infers the cursor from its own writes, which is the
* whole truth only while it is the only thing writing to the terminal. The
* wizard's relay of a piped `hyp sync` is not: that child's send confirm ends
* without a newline, and the answer - with the newline the tty echoes beside
* it - reaches the terminal without passing through here, so the child's next
* diagnostic lands at a real line start this wrap reads as mid-sentence and
* {@link paintChunk}'s gate leaves plain.
*
* A no-op on an unwrapped stream, which is every stream not painting anyway.
*
* @param {{ write(chunk: string): unknown }} stream
*/
export function resyncLineStart(stream) {
const hook = /** @type {any} */ (stream)[RESYNC]
if (typeof hook === 'function') hook()
}

/**
* Wrap a stderr-shaped stream so severity prefixes are coloured on the way
* out, or return it untouched when colour is off.
Expand Down Expand Up @@ -207,6 +230,7 @@ export function colorizeStderr(stream, env) {
/** @type {unknown} */ (stream)
)
let atLineStart = true
const resync = () => { atLineStart = true }
/** @param {unknown} chunk @param {...unknown} rest */
const write = (chunk, ...rest) => {
// Only strings are classified. A Buffer write on stderr is raw bytes
Expand All @@ -221,6 +245,7 @@ export function colorizeStderr(stream, env) {
new Proxy(/** @type {object} */ (/** @type {unknown} */ (stream)), {
get(t, prop, _receiver) {
if (prop === 'write') return write
if (prop === RESYNC) return resync
const value = Reflect.get(t, prop, t)
return typeof value === 'function' ? value.bind(t) : value
},
Expand Down
17 changes: 12 additions & 5 deletions src/core/cli/wizard/sync_now.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
formatFirstSyncDeadline,
readFirstSyncDeadline,
} from '../../usage-policy/first_sync_hold.js'
import { stripSgr } from '../style.js'
import { resyncLineStart, stripSgr } from '../style.js'
import { isTty } from '../tui-router.js'

/**
Expand Down Expand Up @@ -191,10 +191,11 @@ export async function runWizardSyncNow(opts) {
* builds with `terminal: false`, which still writes the query and still reads
* the answer, but takes no raw mode and does no cursor bookkeeping, leaving
* the tty canonical and the terminal itself echoing what is typed; and the
* question ends without a newline, which leaves the parent's `colorizeStderr`
* mid-line, so the next line the child writes reaches the user unpainted.
* Anything that narrows this pipe further has to keep the first of those
* true: the prompt it carries is the one gate on sending.
* question ends without a newline, so the answer - and the newline the tty
* echoes beside it - never passes through the parent's `colorizeStderr`,
* which the echo below resyncs so the child's next diagnostic is still
* classified. Anything that narrows this pipe further has to keep the first
* of those true: the prompt it carries is the one gate on sending.
*
* @ref LLP 0203#child-process [implements]: the release runs in a fresh process so its plan names the real destinations
* @param {RunWizardSyncNowOptions} opts
Expand Down Expand Up @@ -229,8 +230,14 @@ function runSyncChild(opts) {
// needs to be: `close` still fires, so the exit code is still judged,
// only without the corroboration the pipe was there to collect.
child.stderr?.on('error', () => {})
// The tty, not this stream, echoes the answer that ends a question, so
// a chunk following an unterminated one opens a line the echo would
// otherwise read as the middle of that question.
let midLine = false
child.stderr?.on('data', (chunk) => {
const text = String(chunk)
if (midLine) resyncLineStart(echo)
midLine = !text.endsWith('\n')
echo.write(text)
if (noDestinations) return
pending += text
Expand Down
21 changes: 20 additions & 1 deletion test/core/cli/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import test from 'node:test'
import assert from 'node:assert/strict'

import { ANSI, colorizeStderr, paintChunk, paintLine } from '../../../src/core/cli/style.js'
import { ANSI, colorizeStderr, paintChunk, paintLine, resyncLineStart } from '../../../src/core/cli/style.js'

// The CLI severity palette (LLP 0189). Colour is applied once, where
// `dispatch` binds stderr, so these tests pin two things: which leading word
Expand Down Expand Up @@ -181,6 +181,25 @@ test('a write that does not end in a newline leaves the next write mid-line', ()
assert.equal(sink.text(), `${DIM}note:${OFF} error: not a new diagnostic\n`)
})

test('a resync restores the line start the wrap could not see', () => {
// The wizard relays a piped child's stderr: its confirm ends mid-line and
// the tty, not this stream, echoes the answer that ends the line.
const sink = fakeStream(true)
const wrapped = colorizeStderr(sink, {})
wrapped.write('Send now? [Y/n] ')
resyncLineStart(wrapped)
wrapped.write('hyp sync: nothing was sent\n')
assert.equal(sink.text(), `Send now? [Y/n] ${RED}hyp sync:${OFF} nothing was sent\n`)
})

test('a resync on a stream that is not a wrap does nothing', () => {
// The echo target is whatever stderr the caller was handed, which on a
// `NO_COLOR` or piped run is the bare stream.
const sink = fakeStream(false)
resyncLineStart(colorizeStderr(sink, {}))
assert.equal(sink.text(), '')
})

test('the wrap preserves the rest of the stream surface', () => {
// `isTty(stderr)` gates the plugin-install prompt, and the TUI reads
// `columns`. A wrap that hid either would silently degrade them.
Expand Down
27 changes: 26 additions & 1 deletion test/core/cli/wizard/sync_now.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import fs from 'node:fs/promises'
import os from 'node:os'
import path from 'node:path'

import { paintLine } from '../../../../src/core/cli/style.js'
import { ANSI, colorizeStderr, paintLine } from '../../../../src/core/cli/style.js'
import { runWizardSyncNow } from '../../../../src/core/cli/wizard/sync_now.js'
import {
SYNC_HELD_NO_DESTINATIONS_EXIT,
Expand Down Expand Up @@ -354,6 +354,31 @@ test('the child keeps its voice: everything on its stderr is written back out',
assert.equal(o.stderr.text(), 'hyp sync: something broke\n and then more\n')
})

// On a pipe the child cannot paint (`useColor` is false there), so the
// parent's colorized stderr is the only painter left, and the tty echo of the
// answer that ends the confirm never reaches it: without a resync the wrap
// still thinks it is inside the question and the diagnostic arrives plain.
// @ref LLP 0203#child-process [tests]: the relayed child keeps the severity colour it had under inherit
test('the diagnostic after the send confirm keeps its severity colour', async () => {
const spawn = fakeSpawn({
code: 1,
stderr: [
'Send now to the central server? [Y/n] ',
'hyp sync: nothing was sent - the sink driver is holding every tick\n',
],
})
const sink = Object.assign(makeBuf(), { isTTY: true })
const o = opts({ spawnFn: spawn.spawnFn })
o.args.stderr = colorizeStderr(sink, {})
await runWizardSyncNow(o.args)

assert.equal(
sink.text(),
`Send now to the central server? [Y/n] ${ANSI.red}hyp sync:${ANSI.reset} nothing was sent` +
' - the sink driver is holding every tick\n'
)
})

// Piping a stream means owning its failures. An `error` nobody listens for is
// an uncaught exception, and it would land on a setup that had already done
// every one of its acts - the same defect `installStreamErrorHandlers` exists
Expand Down
Loading