fix(install): keep a declined editor on record, and redraw the harness prompts - #4
Open
alifsayalee wants to merge 4 commits into
Open
fix(install): keep a declined editor on record, and redraw the harness prompts#4alifsayalee wants to merge 4 commits into
alifsayalee wants to merge 4 commits into
Conversation
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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
…yped 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 <noreply@anthropic.com>
…ion 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes apimatic/contextmatic-crawler#39.
Two changes, both in the install path.
1. A declined editor stays on record
Re-installing a plugin and declining an editor it was already installed into narrowed the recorded targets without removing the copy.
installedthen reported that editor clean while the editor went on loading the plugin — and sinceupdatereplays the recorded targets, that copy was never refreshed again.Install only ever adds, so the record is now the union of what is on disk: what this run installed, plus the editors an earlier run installed into that this one skipped.
manifest.upsertis deliberately left alone as a plain "make this row say exactly this" primitive —uninstallrelies on that to narrow a row on purpose, so merging insideupsertwould have traded this bug for its mirror image.The closing summary names the skipped editor:
This takes the second of the two fixes #39 offered ("keep the declined target on record and report it accurately"), not the first.
ndoes not delete the copy, and I'd argue it shouldn't: the common reasons to re-runinstallare upgrading via the familiar verb and adding a newly-installed editor. In both you aren't trying to remove anything — so making a decline destructive would turn an absent-mindedninto the loss of a working install.uninstall <plugin> --targets vscodealready does that job correctly (verified: files removed,chat.pluginLocationsunregistered, record narrowed).2. The harness questions read as one flow
The
?sat in a six-column gutter four spaces from its own question, the[Y/n]hint stayed on screen after the decision, and nothing tied the questions to each other or to the result line.Before / after:
The keystroke goes, the question and its hint stay — the hint is what tells you which way a bare Enter would have gone. Rewriting that row needs the cursor, so it runs 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. Both fallbacks leave the row untouched.
Input is unchanged:
y/n/yes/noin any case, bare Enter takes the default.Glyphs come from code points with ASCII stand-ins (
* | +), so a cp437/cp1252 console gets those rather than mojibake — the rulelog.jsalready applies to its check mark.Only the interactive branch changed.
--targets,-y, a non-interactive shell and an injectedconfirmall print exactly what they did before, which is why no existing test needed editing.Verification
Already installed: VS Code, record kept all three.--targetsand via the real prompt code path. Record kept all three, VS Code copy intact at 74 files,settings.jsonentry still present.Not covered by any test: how a real terminal interleaves the row rewrite with readline's own echo. That was eyeballed manually rather than asserted.