fix: skip gha prompt when explicitly set#174
fix: skip gha prompt when explicitly set#174karlhorky wants to merge 3 commits intomicrosoft:mainfrom
Conversation
d4fe561 to
9c6ab6e
Compare
src/generator.ts
Outdated
| { name: 'JavaScript' }, | ||
| ], | ||
| initial: this.options.lang === 'js' ? 'JavaScript' : 'TypeScript', | ||
| initial: this.options.lang === 'js' ? 1 : 0, |
There was a problem hiding this comment.
@Skn0tt this apparently was broken with enquirer for longer, and was causing test failures:
import enquirer from 'enquirer';
const { prompt } = enquirer;
console.log(await prompt([{
type: 'select',
name: 'language',
choices: [{ name: 'TypeScript' }, { name: 'JavaScript' }],
initial: 'JavaScript',
skip: true,
}]));... resolves to { language: 'TypeScript' }
using the choice index instead:
initial: 1... resolves to { language: 'JavaScript' }
There was a problem hiding this comment.
ah that didn't fix the test failure though, trying this enquirer dynamic skip() instead:
fafd255 to
e4e5dce
Compare
| } | ||
|
|
||
| const isDefinitelyTS = fs.existsSync(path.join(this.rootDir, 'tsconfig.json')); | ||
| const cliOptions = this.options; |
There was a problem hiding this comment.
this is needed so that the skip() method below can access this.options from the outer scope but still the this.index from the skip() scope
| run: async ({ packageManager, exec, dir }, use) => { | ||
| await use(async (parameters: string[], options: PromptOptions): Promise<SpawnResult> => { | ||
| await use(async (parameters: string[], options?: PromptOptions): Promise<SpawnResult> => { | ||
| return await exec('node', [path.join(__dirname, '..'), ...parameters], { | ||
| cwd: dir, | ||
| env: { | ||
| ...process.env, | ||
| npm_config_user_agent: userAgents[packageManager], | ||
| 'TEST_OPTIONS': JSON.stringify(options), | ||
| ...(options ? { TEST_OPTIONS: JSON.stringify(options) } : {}), |
There was a problem hiding this comment.
allow omitting the options argument, so that all options can be passed via CLI flags
e4e5dce to
6b96831
Compare
There was a problem hiding this comment.
Pull request overview
Adds explicit control over GitHub Actions workflow generation so that passing --gha or --no-gha is treated as an answered prompt (skipping the interactive question), and updates integration coverage for both quiet and non-quiet flows.
Changes:
- Add
--no-ghaCLI flag and update prompt behavior so explicit--gha/--no-ghaskips the GHA question. - Adjust the language select prompt skip behavior to correctly resolve
--lang jsin the non-quiet path. - Extend integration tests and fixtures to support running the real CLI prompt flow (by omitting
TEST_OPTIONSwhen desired).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tests/integration.spec.ts |
Adds integration coverage for explicit GHA flags in quiet and non-quiet scenarios. |
tests/baseFixtures.ts |
Makes run() prompt overrides optional so tests can exercise real prompt/CLI behavior. |
src/generator.ts |
Treats explicit gha option as answered (skip prompt) and adjusts language selection skip behavior. |
src/cli.ts |
Introduces --no-gha flag to explicitly disable workflow generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .option('--ct', 'install Playwright Component testing') | ||
| .option('--quiet', 'do not ask for interactive input prompts') | ||
| .option('--gha', 'install GitHub Actions') | ||
| .option('--no-gha', 'do not install GitHub Actions') | ||
| .option('--lang <language>', 'language to use (js, TypeScript)') |
There was a problem hiding this comment.
Defining both --gha and --no-gha as separate .option() entries will conflict because Commander derives the same option name (gha) for both (it strips the no- prefix). With Commander’s negated options, this can either throw at startup or (more subtly) make options.gha default to true, which would cause Generator to treat GHA as explicitly answered and skip the prompt / install the workflow even when the user did not pass any GHA flag.
To keep the intended tri-state (undefined when not provided, true for --gha, false for --no-gha), consider using a single negatable option (e.g. --[no-]gha) and then only populating cliOptions.gha when the value source is cli (via command.getOptionValueSource('gha')), otherwise leave it undefined.
There was a problem hiding this comment.
AI slop
without .option('--no-gha', ...), there is no explicit false case to pass through to Generator.
this was tested locally with commander before the change:
- without
.option('--no-gha', ...)- no flag =>
options.gha === undefined --gha=>true--no-gha=> unknown option error
- no flag =>
- with
.option('--no-gha', ...)- no flag =>
options.gha === undefined --gha=>true--no-gha=>false
- no flag =>
so this does not default options.gha to true when no GHA flag is passed
| return false; | ||
| this.index = cliOptions.lang === 'js' ? 1 : 0; | ||
| return true; | ||
| }, |
There was a problem hiding this comment.
the initial string literals apparently was broken with enquirer for longer:
import enquirer from 'enquirer';
const { prompt } = enquirer;
console.log(await prompt([{
type: 'select',
name: 'language',
choices: [{ name: 'TypeScript' }, { name: 'JavaScript' }],
initial: 'JavaScript',
skip: true,
}]));... resolves to { language: 'TypeScript' }
using the choice index instead:
initial: 1... resolves to { language: 'JavaScript' }
first approach: since it caused test failures for this PR, I originally tried to switch to setting initial with a number:
- initial: this.options.lang === 'js' ? 'JavaScript' : 'TypeScript',
+ initial: this.options.lang === 'js' ? 1 : 0,
skip: !!this.options.lang,but that still fails when the prompt is skipped. enquirer never applies the select prompt’s initial during the skipped path, so both forms still resolve to the first choice (TypeScript), which is what was breaking the non-quiet --lang js test here.
so the solution is to use the skip() method form, and set the value via this.index
|
I think the CI failure is some kind of unrelated flakiness: |
alternative to #173
--no-ghato the CLI--gha/--no-ghaas answered so the GitHub Actions prompt is skippedinitialindex for the languageselectprompt so skipped--lang jsresolves correctly in the non-quiet pathContext
This PR is an alternative to #173 for the remaining
--gha/--no-ghaprompt-skipping gap from the linked review comment.