From 517872fad5221d531e060f1d3a276eb8807aea3e Mon Sep 17 00:00:00 2001 From: alifsayalee Date: Fri, 31 Jul 2026 18:03:21 +0500 Subject: [PATCH 1/4] fix(install): keep a declined editor on record instead of dropping it Re-installing a plugin and declining an editor it was already installed into narrowed the recorded targets without removing the copy. `installed` then reported that editor clean while the editor went on loading the plugin, and because `update` replays the recorded targets, that copy was never refreshed again - a stale plugin with no visibility into it. Install only ever adds, so the record has to be the union of what is on disk: what this run installed, plus the editors an earlier run installed into that this one skipped. `manifest.upsert` stays a plain "make this row say exactly this" primitive - uninstall relies on that to narrow a row deliberately, so merging inside upsert would have traded this bug for its mirror image. The closing summary names the skipped editor ("Already installed: VS Code"), since that is where the user looks to see where the plugin now lives. Not covered here: making a decline actually remove the copy. That needs the prompt reworded (a decline should not read as a silent deletion) and a guard so a --targets subset in a script cannot delete unlisted editors. Refs apimatic/contextmatic-crawler#39 Co-Authored-By: Claude Opus 5 --- src/install.js | 31 ++++++++++++++--- test/install.test.js | 79 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/install.js b/src/install.js index 3fb80c3..42d7cea 100644 --- a/src/install.js +++ b/src/install.js @@ -114,6 +114,7 @@ async function runInstall({ brand, plugin, ref, targets, force, assumeYes, deps, const requested = resolveTargets(targets); assertNoMarketplaceConflict(manifestFile, { plugin, repo: brand.repo }, force); + const recorded = manifest.find(manifestFile, { plugin, repo: brand.repo }); const from = effectiveRef === 'main' ? brand.label : `${brand.label} (${effectiveRef})`; log.banner(`Installing '${plugin}' from ${from}`); @@ -163,6 +164,12 @@ async function runInstall({ brand, plugin, ref, targets, force, assumeYes, deps, } log.info(`Installing into: ${want.map((n) => byName(n).title).join(', ')}`); + // Editors this run skipped that an earlier one installed into. Install only ever + // adds, so their copies are still on disk untouched - they stay on the record + // below (or `update` would never refresh them again) and get a line in the + // closing summary, which is where the user looks to see where the plugin lives. + const untouched = (recorded ? recorded.targets || [] : []).filter((n) => !want.includes(n)); + // Only pay for the fetch if a chosen harness needs the files. The session // owns the clone, so a second plugin from the same repo checks out locally. const needsSource = want.some((name) => byName(name).needsSource); @@ -196,19 +203,30 @@ async function runInstall({ brand, plugin, ref, targets, force, assumeYes, deps, } if (installed.length) { + // The record is the union of what is on disk: what this run installed, plus the + // editors an earlier run installed into that this one left alone. Writing only + // `installed` would drop those, and `update` reads this list to decide what to + // refresh - so a dropped editor becomes a copy that is never updated again. + const keep = new Set([...untouched, ...installed]); manifest.upsert(manifestFile, { plugin, repo: brand.repo, marketplace: resolved.marketplace, ref: effectiveRef, - targets: installed, + targets: NAMES.filter((n) => keep.has(n)), // canonical order, not call order installedAt: nowIso(), }); } - summarize(installed, 'Installed into'); + summarize(installed, 'Installed into', untouched); - return { plugin, targets: installed, marketplace: resolved.marketplace, ref: effectiveRef }; + return { + plugin, + targets: installed, + untouched, + marketplace: resolved.marketplace, + ref: effectiveRef, + }; } async function uninstallPlugin({ brand, plugin, targets, deps = {}, pathOpts } = {}) { @@ -346,7 +364,7 @@ async function listPlugins({ brand, deps = {}, pathOpts } = {}) { }; } -function summarize(done, verb) { +function summarize(done, verb, unchanged = []) { log.plain(''); log.rule(); if (!done.length) { @@ -354,6 +372,11 @@ function summarize(done, verb) { } else { log.ok(`${verb}: ${done.map((n) => byName(n).title).join(', ')}`); } + // An editor that already had the plugin and was skipped this run still has it, so + // it belongs in the report - otherwise this reads as "it is only in these two". + if (unchanged.length) { + log.info(`Already installed: ${unchanged.map((n) => byName(n).title).join(', ')}`); + } log.plain(''); } diff --git a/test/install.test.js b/test/install.test.js index 5b4c9f2..4b6c4bf 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -339,6 +339,85 @@ test('a declined harness is not touched', async () => { assert.deepEqual(manifest.list(paths.manifestPath(m.pathOpts))[0].targets, ['vscode']); }); +test('declining an editor it is ALREADY installed in keeps the record and the files', async () => { + const m = machine(); + const repo = 'context-plugins/plugin-marketplace'; + const srcDir = pluginSource(); + const d = deps({ repo, srcDir }); + const args = { brand: brandFor(repo), plugin: 'my-sdk', deps: d, pathOpts: m.pathOpts }; + const vscodeDest = path.join(m.pathOpts.env.CP_STATE_DIR, 'vscode', 'my-sdk'); + const settingsFile = path.join(m.pathOpts.env.CP_VSCODE_USER_DIR, 'settings.json'); + + // Installed into both to begin with. + await quietly(() => installPlugin({ ...args, targets: TARGETS })); + assert.deepEqual(manifest.list(paths.manifestPath(m.pathOpts))[0].targets, ['cursor', 'vscode']); + + // Re-install, saying yes to Cursor and no to VS Code. + const result = await quietly(() => + installPlugin({ ...args, targets: null, deps: { ...d, confirm: scriptedConfirm([true, false]) } }), + ); + + assert.deepEqual(result.targets, ['cursor'], 'only Cursor was installed into this run'); + assert.deepEqual(result.untouched, ['vscode'], 'VS Code reported as left alone'); + + // The record still names VS Code, so `update` keeps refreshing that copy. + assert.deepEqual( + manifest.list(paths.manifestPath(m.pathOpts))[0].targets, + ['cursor', 'vscode'], + 'the declined editor stays on record', + ); + + // And the declined copy is genuinely untouched, not removed. + assert.ok(fs.existsSync(path.join(vscodeDest, 'plugin.json')), 'VS Code files still there'); + const settings = parseJsonc(fs.readFileSync(settingsFile, 'utf8')); + assert.equal(settings['chat.pluginLocations'][vscodeDest.replace(/\\/g, '/')], true, 'still registered'); +}); + +test('the declined-but-installed editor is named once, in the summary', async () => { + const m = machine(); + const repo = 'context-plugins/plugin-marketplace'; + const srcDir = pluginSource(); + const d = deps({ repo, srcDir }); + const args = { brand: brandFor(repo), plugin: 'my-sdk', deps: d, pathOpts: m.pathOpts }; + + await quietly(() => installPlugin({ ...args, targets: TARGETS })); + + const con = silenceConsole(); + try { + await installPlugin({ ...args, targets: null, deps: { ...d, confirm: scriptedConfirm([true, false]) } }); + } finally { + con.restore(); + } + // Colour is off without a TTY, but strip it anyway so FORCE_COLOR cannot break this. + const out = con.lines.join('\n').replace(/\x1b\[\d+m/g, ''); + + assert.match(out, /Already installed: VS Code/); + // One line, in the summary - nothing up in [Harnesses]. The earlier version said it + // twice and ran to four wrapped lines, which buried the install report itself. + assert.equal(out.match(/Already installed/g).length, 1, 'said exactly once'); + assert.doesNotMatch(out, /not removed/); + assert.doesNotMatch(out, /--targets vscode/); +}); + +test('a fresh install records only what it installed', async () => { + const m = machine(); + const repo = 'context-plugins/plugin-marketplace'; + const srcDir = pluginSource(); + + await quietly(() => + installPlugin({ + brand: brandFor(repo), + plugin: 'my-sdk', + targets: null, + deps: { ...deps({ repo, srcDir }), confirm: scriptedConfirm([true, false]) }, + pathOpts: m.pathOpts, + }), + ); + + // No prior record, so nothing to preserve - the union must not invent a target. + assert.deepEqual(manifest.list(paths.manifestPath(m.pathOpts))[0].targets, ['cursor']); +}); + test('declining everything changes nothing at all', async () => { const m = machine(); const repo = 'context-plugins/plugin-marketplace'; From 4b0bd892fafde233adb7c052601ab2cfa8c0ec90 Mon Sep 17 00:00:00 2001 From: alifsayalee Date: Fri, 31 Jul 2026 19:26:50 +0500 Subject: [PATCH 2/4] feat(prompt): draw the harness questions as one flow, not three stray lines The `?` sat in a six-column gutter, four spaces from its own question, so there was nothing for the eye to anchor to; the `[Y/n]` hint stayed on screen after the decision was made; and nothing tied the three questions to each other or to the result line they produce. Redrawn on the pattern Clack established: the glyph sits against its text with a single gap, a connector runs from each answer down to the next question and into the closing line, and the hint is replaced by the decision itself once there is one - a resolved step reads "Yes", not "(Y/n) y". Rewriting the asked row needs the cursor, so it is attempted only on a TTY and only when the row cannot have wrapped; past the terminal width the arithmetic would clear the wrong line and eat real output. Off a TTY nothing echoes the user's Enter, so the newline has to be written instead of assumed. Answers still take y/n as well as yes/no, in any case, and a bare Enter still takes the default. Only the interactive branch changes: --targets, -y, a non-interactive shell and an injected `confirm` all print exactly what they did before, which is why the existing suite needed no edits. Glyphs come from code points and carry ASCII stand-ins, so a cp437/cp1252 console gets `*` and `|` rather than mojibake - the rule log.js already applies to its check mark. Co-Authored-By: Claude Opus 5 --- src/install.js | 24 +++++++++++--- src/log.js | 10 ++++++ src/prompt.js | 79 ++++++++++++++++++++++++++++++++++++++------- test/prompt.test.js | 51 +++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 16 deletions(-) create mode 100644 test/prompt.test.js diff --git a/src/install.js b/src/install.js index 42d7cea..01c8fcb 100644 --- a/src/install.js +++ b/src/install.js @@ -26,7 +26,10 @@ async function askEach(names, ask) { * --targets is a decision, --yes opts out, and a non-interactive shell has * nobody to ask (so it uses every detected assistant rather than hanging). */ -async function chooseHarnesses(available, { explicit = false, assumeYes = false, confirm } = {}) { +async function chooseHarnesses( + available, + { explicit = false, assumeYes = false, confirm, onPrompted } = {}, +) { if (!available.length || explicit || assumeYes) return available; if (confirm) return askEach(available, confirm); @@ -36,6 +39,9 @@ async function chooseHarnesses(available, { explicit = false, assumeYes = false, return available; } + // Only this branch draws the prompt flow, so only this branch leaves a connector + // for the caller to close with `log.groupEnd`. + if (onPrompted) onPrompted(); const prompter = createPrompter(); try { return await askEach(available, (question, def) => prompter.confirm(question, def)); @@ -152,17 +158,27 @@ async function runInstall({ brand, plugin, ref, targets, force, assumeYes, deps, log.info(`Continuing with ${available.map((n) => byName(n).title).join(', ')}.`); } + let prompted = false; const want = await chooseHarnesses(available, { explicit, assumeYes, confirm: deps.confirm, + onPrompted: () => { + prompted = true; + }, }); + // When the questions were drawn, this line closes their flow; otherwise nothing was + // drawn to close and it stays the plain info line it has always been. + const closeGroup = (msg) => (prompted ? log.groupEnd(msg) : log.info(msg)); if (!want.length) { - log.plain(''); - log.warn('No harness selected - nothing was installed.'); + if (prompted) log.groupEnd('No harness selected - nothing was installed.'); + else { + log.plain(''); + log.warn('No harness selected - nothing was installed.'); + } return { plugin, targets: [], marketplace: resolved.marketplace, ref: effectiveRef }; } - log.info(`Installing into: ${want.map((n) => byName(n).title).join(', ')}`); + closeGroup(`Installing into: ${want.map((n) => byName(n).title).join(', ')}`); // Editors this run skipped that an earlier one installed into. Install only ever // adds, so their copies are still on disk untouched - they stay on the record diff --git a/src/log.js b/src/log.js index d302333..b4faa10 100644 --- a/src/log.js +++ b/src/log.js @@ -25,6 +25,9 @@ const TICK = unicodeSupported() ? ` ${String.fromCharCode(0x2713)} ` : ' OK const BANG = ' !! '; const CROSS = ' XX '; const MARK = unicodeSupported() ? String.fromCharCode(0x2713) : '*'; +// Closes the prompt flow drawn by prompt.js, so its connector has somewhere to land. +// Its 3-column gutter matches that flow's, not the 6 of the prefixes above. +const GROUP_END = unicodeSupported() ? String.fromCharCode(0x2514) : '+'; /** * Terminal width, clamped: prose past ~78 columns is harder to read, not easier. @@ -133,6 +136,13 @@ const log = { console.log(`${paint('33', BANG)}${first}`); for (const line of rest) console.log(paint('33', ` ${line}`)); }, + /** The last line of the prompt flow: `└ `, closing the connector above it. */ + groupEnd(msg) { + if (state.quiet) return; + const [first, ...rest] = wrap(ascii(msg), 3); + console.log(`${paint('90', GROUP_END)} ${first}`); + for (const line of rest) console.log(` ${line}`); + }, error(msg) { const [first, ...rest] = wrap(ascii(msg)); console.error(`${paint('31', CROSS)}${first}`); diff --git a/src/prompt.js b/src/prompt.js index 1d37b51..791f3f1 100644 --- a/src/prompt.js +++ b/src/prompt.js @@ -2,6 +2,7 @@ const readline = require('node:readline/promises'); const { stdin, stdout } = require('node:process'); +const log = require('./log'); /** * A prompt is only safe when a human is actually there. Piped input, CI, and @@ -17,37 +18,91 @@ function isInteractive(env = process.env) { const YES = new Set(['y', 'yes']); const NO = new Set(['n', 'no']); -function createPrompter() { - const rl = readline.createInterface({ input: stdin, output: stdout }); +const ESC = String.fromCharCode(27); +const UP_AND_CLEAR = `${ESC}[1A${ESC}[2K\r`; + +/** + * The questions in one run are a single flow, not a handful of unrelated lines, so + * they are drawn as one: the glyph sits against its text with no floating gap, and a + * connector runs from each answer down to the next question and on to whatever the + * caller prints last (see `log.groupEnd`). + * + * Legacy Windows consoles run cp437/cp1252 and render box drawing as mojibake, so + * every glyph has an ASCII stand-in - the same rule log.js applies to its check mark. + * Built from code points rather than pasted in, so the source stays ASCII and no + * editor or console re-encoding can turn them into mojibake either. + */ +function glyphs(unicode = log.unicodeSupported()) { + return unicode + ? { step: String.fromCharCode(0x25c6), bar: String.fromCharCode(0x2502) } // diamond, bar + : { step: '*', bar: '|' }; +} + +/** + * `true`/`false` for an answer, `null` for anything else so the caller can re-ask. + * Empty input takes the default, which is what makes a bare Enter work. + */ +function parseAnswer(input, defaultYes = true) { + if (input === undefined || input === null) return defaultYes; + const normalized = String(input).trim().toLowerCase(); + if (normalized === '') return defaultYes; + if (YES.has(normalized)) return true; + if (NO.has(normalized)) return false; + return null; +} + +// `input`/`out`/`unicode` default to the real terminal; they are overridable so the +// flow can be driven and inspected without one. +function createPrompter({ input = stdin, out = stdout, unicode } = {}) { + const g = glyphs(unicode); + const rl = readline.createInterface({ input, output: out }); rl.on('SIGINT', () => { rl.close(); - stdout.write('\nCancelled.\n'); + out.write(`\n${g.bar} Cancelled.\n`); process.exit(130); }); + // Redrawing means clearing the row the user just typed on. Only attempt it on a + // TTY, and only when that row cannot have wrapped - past the terminal width the + // cursor arithmetic would clear the wrong line and eat real output. + const canRedraw = (line) => Boolean(out.isTTY) && line.length < (out.columns || 80); + return { async confirm(question, defaultYes = true) { - const suffix = defaultYes ? '[Y/n]' : '[y/N]'; + const hint = defaultYes ? '(Y/n)' : '(y/N)'; + const asked = `${g.step} ${question} ${hint} `; for (let attempt = 0; attempt < 3; attempt += 1) { let answer; try { - answer = await rl.question(` ? ${question} ${suffix} `); + answer = await rl.question(asked); } catch { return defaultYes; // stdin closed mid-question } - if (answer === undefined) return defaultYes; - const normalized = answer.trim().toLowerCase(); - if (normalized === '') return defaultYes; - if (YES.has(normalized)) return true; - if (NO.has(normalized)) return false; - stdout.write(' Please answer y or n.\n'); + const parsed = parseAnswer(answer, defaultYes); + if (parsed === null) { + out.write(`${log.dim(g.bar)} Please answer yes or no.\n`); + continue; + } + // The hint is noise once the decision is made: redraw the asked row as the + // question alone, then put the decision under it as its own resolved step. + if (canRedraw(asked + String(answer))) { + out.write(UP_AND_CLEAR); + out.write(`${log.dim(g.step)} ${question}\n`); + } else if (!out.isTTY) { + // A TTY echoes the user's Enter, so the cursor is already on a fresh row. + // Nothing echoes off one, so the answer would land on the asked row. + out.write('\n'); + } + out.write(`${log.dim(g.bar)} ${log.dim(parsed ? 'Yes' : 'No')}\n`); + return parsed; } return defaultYes; }, close() { + out.write(`${log.dim(g.bar)}\n`); // connector into the caller's closing line rl.close(); }, }; } -module.exports = { isInteractive, createPrompter }; +module.exports = { isInteractive, createPrompter, glyphs, parseAnswer }; diff --git a/test/prompt.test.js b/test/prompt.test.js new file mode 100644 index 0000000..2597116 --- /dev/null +++ b/test/prompt.test.js @@ -0,0 +1,51 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); + +const { glyphs, parseAnswer, isInteractive } = require('../src/prompt'); + +test('y, n, and their long forms are all accepted', () => { + for (const yes of ['y', 'Y', 'yes', 'YES', ' Yes ']) { + assert.equal(parseAnswer(yes, true), true, `${JSON.stringify(yes)} should be yes`); + } + for (const no of ['n', 'N', 'no', 'NO', ' No ']) { + assert.equal(parseAnswer(no, true), false, `${JSON.stringify(no)} should be no`); + } +}); + +test('bare Enter takes the default, either way round', () => { + assert.equal(parseAnswer('', true), true); + assert.equal(parseAnswer('', false), false); + assert.equal(parseAnswer(' ', true), true); +}); + +test('stdin closing mid-question falls back to the default', () => { + assert.equal(parseAnswer(undefined, true), true); + assert.equal(parseAnswer(null, false), false); +}); + +test('anything else is null, so the caller re-asks instead of guessing', () => { + for (const junk of ['maybe', 'ye', 'yep', 'nope', '1', 'true']) { + assert.equal(parseAnswer(junk, true), null, `${junk} should not be taken as an answer`); + } +}); + +test('the glyphs fall back to ASCII where box drawing would be mojibake', () => { + const uni = glyphs(true); + const ascii = glyphs(false); + assert.equal(uni.step, String.fromCharCode(0x25c6)); + assert.equal(uni.bar, String.fromCharCode(0x2502)); + assert.equal(ascii.step, '*'); + assert.equal(ascii.bar, '|'); + // One column each, so the 3-column gutter lines up in both modes. + for (const g of [uni, ascii]) { + assert.equal(g.step.length, 1); + assert.equal(g.bar.length, 1); + } +}); + +test('CI and CP_NO_INPUT both force non-interactive', () => { + assert.equal(isInteractive({ CI: '1' }), false); + assert.equal(isInteractive({ CP_NO_INPUT: '1' }), false); +}); From 652e67c895226e146b171314f9c54ec7ab86689f Mon Sep 17 00:00:00 2001 From: alifsayalee Date: Fri, 31 Jul 2026 19:34:47 +0500 Subject: [PATCH 3/4] refactor(prompt): leave the answered question on screen as the user typed it Rewriting the asked row to drop its `(Y/n)` once answered read as a flicker: the line the user was just looking at changes under them. Keep it exactly as typed. Dropping the rewrite also removes the cursor arithmetic behind it. That was the one part of this flow that could destroy output rather than just look wrong - a row cleared one line off target takes real text with it - and it needed a TTY to verify, which no test could give it. The flow now only ever grows downwards, so there is nothing left to get wrong. Co-Authored-By: Claude Opus 5 --- src/prompt.js | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/src/prompt.js b/src/prompt.js index 791f3f1..e9fdf4b 100644 --- a/src/prompt.js +++ b/src/prompt.js @@ -18,14 +18,12 @@ function isInteractive(env = process.env) { const YES = new Set(['y', 'yes']); const NO = new Set(['n', 'no']); -const ESC = String.fromCharCode(27); -const UP_AND_CLEAR = `${ESC}[1A${ESC}[2K\r`; - /** * The questions in one run are a single flow, not a handful of unrelated lines, so * they are drawn as one: the glyph sits against its text with no floating gap, and a * connector runs from each answer down to the next question and on to whatever the - * caller prints last (see `log.groupEnd`). + * caller prints last (see `log.groupEnd`). Nothing already on screen is ever + * rewritten - the flow only ever grows downwards. * * Legacy Windows consoles run cp437/cp1252 and render box drawing as mojibake, so * every glyph has an ASCII stand-in - the same rule log.js applies to its check mark. @@ -62,11 +60,6 @@ function createPrompter({ input = stdin, out = stdout, unicode } = {}) { process.exit(130); }); - // Redrawing means clearing the row the user just typed on. Only attempt it on a - // TTY, and only when that row cannot have wrapped - past the terminal width the - // cursor arithmetic would clear the wrong line and eat real output. - const canRedraw = (line) => Boolean(out.isTTY) && line.length < (out.columns || 80); - return { async confirm(question, defaultYes = true) { const hint = defaultYes ? '(Y/n)' : '(y/N)'; @@ -83,16 +76,13 @@ function createPrompter({ input = stdin, out = stdout, unicode } = {}) { out.write(`${log.dim(g.bar)} Please answer yes or no.\n`); continue; } - // The hint is noise once the decision is made: redraw the asked row as the - // question alone, then put the decision under it as its own resolved step. - if (canRedraw(asked + String(answer))) { - out.write(UP_AND_CLEAR); - out.write(`${log.dim(g.step)} ${question}\n`); - } else if (!out.isTTY) { - // A TTY echoes the user's Enter, so the cursor is already on a fresh row. - // Nothing echoes off one, so the answer would land on the asked row. - out.write('\n'); - } + // The asked row is left exactly as the user saw it, hint and keystroke and + // all. Rewriting it to drop the hint reads as a flicker, and it would mean + // clearing a row with the cursor - which eats real output the moment the + // arithmetic is off by one. + // A TTY echoes the user's Enter, so the cursor is already on a fresh row. + // Nothing echoes off one, so the answer would land on the asked row. + if (!out.isTTY) out.write('\n'); out.write(`${log.dim(g.bar)} ${log.dim(parsed ? 'Yes' : 'No')}\n`); return parsed; } From 1950852d441e12142f192d11f969bda12b969315 Mon Sep 17 00:00:00 2001 From: alifsayalee Date: Fri, 31 Jul 2026 19:38:25 +0500 Subject: [PATCH 4/4] feat(prompt): drop the keystroke from an answered row, keep the question and hint Reading back `(Y/n) y` on the question and `Yes` under it is the same answer twice, and the hint is what tells you which way a bare Enter would have gone - so the keystroke is the part that goes, not the hint. The row is redrawn as it was asked. That needs the cursor, so it is attempted only on a TTY and only when the row cannot have wrapped: past the terminal width the arithmetic would clear the wrong line and take real output with it. Off a TTY, or on a row that might have wrapped, nothing is touched. Two tests cover the contract headless, against a sink that reports itself as a terminal - that the cleared row comes back with its hint and without the keystroke, and that neither a narrow terminal nor a pipe emits cursor codes at all. What they cannot cover is how a real terminal interleaves this with readline's own echo; that still wants an eye on it. Co-Authored-By: Claude Opus 5 --- src/prompt.js | 29 ++++++++++++++------- test/prompt.test.js | 61 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 10 deletions(-) diff --git a/src/prompt.js b/src/prompt.js index e9fdf4b..cfe4860 100644 --- a/src/prompt.js +++ b/src/prompt.js @@ -18,12 +18,14 @@ function isInteractive(env = process.env) { const YES = new Set(['y', 'yes']); const NO = new Set(['n', 'no']); +const ESC = String.fromCharCode(27); +const UP_AND_CLEAR = `${ESC}[1A${ESC}[2K\r`; + /** * The questions in one run are a single flow, not a handful of unrelated lines, so * they are drawn as one: the glyph sits against its text with no floating gap, and a * connector runs from each answer down to the next question and on to whatever the - * caller prints last (see `log.groupEnd`). Nothing already on screen is ever - * rewritten - the flow only ever grows downwards. + * caller prints last (see `log.groupEnd`). * * Legacy Windows consoles run cp437/cp1252 and render box drawing as mojibake, so * every glyph has an ASCII stand-in - the same rule log.js applies to its check mark. @@ -60,6 +62,11 @@ function createPrompter({ input = stdin, out = stdout, unicode } = {}) { process.exit(130); }); + // Redrawing means clearing the row the user just typed on. Only attempt it on a + // TTY, and only when that row cannot have wrapped - past the terminal width the + // cursor arithmetic would clear the wrong line and eat real output. + const canRedraw = (line) => Boolean(out.isTTY) && line.length < (out.columns || 80); + return { async confirm(question, defaultYes = true) { const hint = defaultYes ? '(Y/n)' : '(y/N)'; @@ -76,13 +83,17 @@ function createPrompter({ input = stdin, out = stdout, unicode } = {}) { out.write(`${log.dim(g.bar)} Please answer yes or no.\n`); continue; } - // The asked row is left exactly as the user saw it, hint and keystroke and - // all. Rewriting it to drop the hint reads as a flicker, and it would mean - // clearing a row with the cursor - which eats real output the moment the - // arithmetic is off by one. - // A TTY echoes the user's Enter, so the cursor is already on a fresh row. - // Nothing echoes off one, so the answer would land on the asked row. - if (!out.isTTY) out.write('\n'); + // The keystroke goes, the question and its hint stay: the row is redrawn as it + // was asked, and the decision lands under it as its own resolved step. Reading + // back `(Y/n) y` next to `Yes` is the same answer twice. + if (canRedraw(asked + String(answer))) { + out.write(UP_AND_CLEAR); + out.write(`${log.dim(g.step)} ${question} ${hint}\n`); + } else if (!out.isTTY) { + // A TTY echoes the user's Enter, so the cursor is already on a fresh row. + // Nothing echoes off one, so the answer would land on the asked row. + out.write('\n'); + } out.write(`${log.dim(g.bar)} ${log.dim(parsed ? 'Yes' : 'No')}\n`); return parsed; } diff --git a/test/prompt.test.js b/test/prompt.test.js index 2597116..50a92a3 100644 --- a/test/prompt.test.js +++ b/test/prompt.test.js @@ -3,7 +3,39 @@ const test = require('node:test'); const assert = require('node:assert'); -const { glyphs, parseAnswer, isInteractive } = require('../src/prompt'); +const { PassThrough, Writable } = require('stream'); + +const { glyphs, parseAnswer, isInteractive, createPrompter } = require('../src/prompt'); + +const ESC = String.fromCharCode(27); +const UP_AND_CLEAR = `${ESC}[1A${ESC}[2K\r`; + +/** A sink that reports itself as a terminal, so the redraw path runs headless. */ +function fakeTty({ isTTY = true, columns = 80 } = {}) { + let buf = ''; + const s = new Writable({ + write(c, _e, cb) { + buf += c.toString(); + cb(); + }, + }); + s.isTTY = isTTY; + s.columns = columns; + s.text = () => buf; + return s; +} + +/** Asks one question, answering with `keys`. Returns [answer, what was written]. */ +async function askOnce(keys, { out, question = 'Install into VS Code?' } = {}) { + const input = new PassThrough(); + const prompter = createPrompter({ input, out, unicode: true }); + const pending = prompter.confirm(question, true); + input.write(`${keys}\n`); + const answer = await pending; + prompter.close(); + input.end(); + return [answer, out.text()]; +} test('y, n, and their long forms are all accepted', () => { for (const yes of ['y', 'Y', 'yes', 'YES', ' Yes ']) { @@ -45,6 +77,33 @@ test('the glyphs fall back to ASCII where box drawing would be mojibake', () => } }); +test('the answered row is redrawn with its hint, minus the keystroke', async () => { + const out = fakeTty(); + const [answer, text] = await askOnce('y', { out }); + const g = glyphs(true); + + assert.equal(answer, true); + const at = text.indexOf(UP_AND_CLEAR); + assert.ok(at !== -1, 'the row the user typed on is cleared'); + // What replaces it keeps the question AND the hint - only the keystroke goes. + const after = text.slice(at + UP_AND_CLEAR.length); + assert.match(after, /^.*Install into VS Code\? \(Y\/n\)\n/); + assert.ok(!/\(Y\/n\)\s+y/.test(after), 'the keystroke is not carried into the redraw'); + // Then the decision, on its own connector row. + assert.match(after, new RegExp(`\\${g.bar}\\s+Yes\\n`)); +}); + +test('no cursor tricks when the row could have wrapped, or off a TTY', async () => { + const narrow = fakeTty({ columns: 10 }); // the asked row cannot fit + const [, wrapped] = await askOnce('y', { out: narrow }); + assert.ok(!wrapped.includes(UP_AND_CLEAR), 'a row that may have wrapped is left alone'); + + const piped = fakeTty({ isTTY: false }); + const [, plain] = await askOnce('n', { out: piped }); + assert.ok(!plain.includes(UP_AND_CLEAR), 'nothing to redraw when there is no terminal'); + assert.match(plain, /\n.*No\n/, 'the answer still lands on its own row'); +}); + test('CI and CP_NO_INPUT both force non-interactive', () => { assert.equal(isInteractive({ CI: '1' }), false); assert.equal(isInteractive({ CP_NO_INPUT: '1' }), false);