fix: error on non-interactive terminals without --quiet#173
fix: error on non-interactive terminals without --quiet#173Skn0tt wants to merge 2 commits intomicrosoft:mainfrom
Conversation
| if (!process.stdin.isTTY) { | ||
| console.error('Non-interactive terminal detected. See --help for options, then run again with --quiet.'); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
@Skn0tt hmm this seems like an antipattern 🤔
- after fix: ensure non-TTY agents dont hang #172,
--quietis only one way to pass in all options. if options are passed in, the questions could be skipped below - if I remember correctly,
--quietoverrides all options passed in (which makes it less useful)
actually with all of the flags, --quiet isn't really needed, unless you want to silence the stdout (that's probably what a --quiet flag should do, not modify any options or read anything related to TTY)
if the enquirer questions are skipped, the --quiet flag is no longer needed. it's a simpler mental model - if you pass all answers via flags, it skips all questions
There was a problem hiding this comment.
It respects all options passed in: https://github.com/Skn0tt/create-playwright/blob/88b15eb00ff1d63618791b4cfe54da6b40bfdb3d/src/generator.ts#L90-L95
Can you elaborate on what you think will be broken by this?
There was a problem hiding this comment.
what would be broken: passing in all answers via flags without the --quiet flag
it's less obvious what this does:
pnpm create playwright --browser=chromium --test-dir=playwright --quietit would be easier to understand if you can see the option in the command (either of these would break after your PR):
pnpm create playwright --browser=chromium --test-dir=playwright --no-gha
pnpm create playwright --browser=chromium --test-dir=playwright --ghathat's why I was asking for a --no-gha flag, to be able to just exhaustively pass in all options
once you can exhaustively pass all options (and they skip the questions), then it works non-interactively already, and the --quiet flag can be dropped / deprecated
There was a problem hiding this comment.
@Skn0tt alternative PR of what I mean with my proposal:
There was a problem hiding this comment.
Thanks for elaborating. With the approach you're outlining, some agent implementations will start hanging if they forget both --no-gha and --gha. My approach where non-interactive terminals need to use --quiet is a little more cumbersome for humans with non-interactive terminals, but one less footgun for agents. Since humans use interactive terminals, this seems to be the right approach.
There was a problem hiding this comment.
Hm, but the --quiet flag is worse UX for humans
some agent implementations will start hanging if they forget both
--no-ghaand--gha
Good point - so my PR #174 could check for non-interactive (same as your PR) and then short-circuit like --quiet currently does
Better behavior than erroring out for both AI and humans
Edit: added over in my PR #174
When a non-interactive terminal (e.g. a coding agent spawning with
stdio: ['pipe', ...]) runscreate-playwrightwithout--quiet, enquirer prompts hang indefinitely because stdin never closes.This adds a
process.stdin.isTTYcheck after the--quietearly return. If the terminal is non-interactive and--quietwasn't passed, we print an actionable error message and exit instead of hanging.Split out from #172 for easier revert.