-
Notifications
You must be signed in to change notification settings - Fork 0
fix(setup): preserve interactive init and cancellation #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,7 @@ export async function main(argv = process.argv) { | |
| await runSetup(); | ||
| break; | ||
| case "init": | ||
| await runSetup({ defaults: true }); | ||
| await runSetup(); | ||
| break; | ||
|
Comment on lines
31
to
33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this change, the This redundancy makes the distinction between If |
||
| case "append": | ||
| await runAppend(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,4 +52,12 @@ describe("shell utilities", () => { | |
| expect(brewInstall("ghostty", { cask: true })).toBe(true); | ||
| expect(execSync).toHaveBeenCalledWith("brew install --no-ask --cask ghostty", { stdio: "inherit" }); | ||
| }); | ||
|
|
||
| test("propagates Ctrl-C from Homebrew installs", () => { | ||
| execSync.mockImplementation(() => { | ||
| throw Object.assign(new Error("interrupted"), { signal: "SIGINT", status: null }); | ||
| }); | ||
|
|
||
| expect(() => brewInstall("jq")).toThrow(expect.objectContaining({ interrupted: true })); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Vitest (and Jest), passing an asymmetric matcher like To reliably assert custom properties on a thrown error, you should catch the error and assert its properties directly. let thrownError;
try {
brewInstall("jq");
} catch (error) {
thrownError = error;
}
expect(thrownError).toBeDefined();
expect(thrownError.interrupted).toBe(true); |
||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the
initcommand has been updated insrc/cli.jsto callrunSetup()without any arguments, it now behaves identically to thesetupcommand.Presenting these as two distinct options here (with different descriptions) is misleading to the user, as both choices will launch the exact same interactive setup flow. Consider removing this prompt and defaulting to a single setup flow, or updating the descriptions to reflect that they are now identical.