diff --git a/src/install.js b/src/install.js index 3fb80c3..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)); @@ -114,6 +120,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}`); @@ -151,17 +158,33 @@ 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 + // 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. @@ -196,19 +219,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 +380,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 +388,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/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..cfe4860 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,92 @@ 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 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; } 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/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'; diff --git a/test/prompt.test.js b/test/prompt.test.js new file mode 100644 index 0000000..50a92a3 --- /dev/null +++ b/test/prompt.test.js @@ -0,0 +1,110 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); + +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 ']) { + 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('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); +});