Repository navigation
Open a yes/no prompt on its default, so Enter picks it - #802
Merged
Merged
Conversation
tui.Confirm took a default but never used it: the answer started as false, so every prompt opened on No, and pressing Enter answered No even where the question meant Yes. `basecamp setup` asks "Set up Basecamp for your coding agents?" and "Set a default project?" that way, and guided `connect setup` asks "Work in all of your agent's projects?"; pressing Enter there ended with no projects chosen. The prompt now opens on the default it's given. Prompts that default to No, like the skill overwrite, behave as before.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The test bypasses the prompt, so it does not verify what Enter selects.
Review effort: Balanced
Findings: 1
What changed in this PR
This PR updates the CLI’s shared yes/no prompt so Enter selects the supplied default, addressing setup prompts that unintentionally answered No.
Changes:
- Initialize confirmation prompts from their default value.
- Add a test for both Yes and No defaults.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
internal/tui/forms.go |
Initializes the prompt answer from its default. |
internal/tui/forms_test.go |
Adds a test for both default values. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
With TERM=dumb, huh runs forms in accessible mode, which answers end of input (Ctrl+D) with the prompt's default and can't tell it from Enter. Now that a prompt opens on its default, a question that defaults to Yes, like "Set up Basecamp for your coding agents?", would take Ctrl+D as consent. In accessible mode the prompt opens on No and a Yes must be typed. The test now answers through the field's own input handling, typed answers against the default included, so a prompt that isn't bound to Confirm's result fails it.
The earlier tests answer through huh's text-input path. This one drives Confirm's field with the key map runForm gives every form, as the interactive prompt does: Enter submits what the prompt opened on, y and n answer, and the arrows toggle. It sets TERM itself, so it means the same on a CI runner whose terminal is dumb. bubbletea v1 was already in the module graph; the test's import makes it a direct requirement.
Enter leaves a prompt's value as it opened, so a test asserting only the value passed even with Enter unbound. The keyboard test now also checks that the last key of each answer submits the field.
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.
When the CLI asks a yes/no question, pressing Enter picks No, even when the question means Yes.
tui.Confirm(message, defaultValue)never used its default: the answer started asfalse, so every prompt opened on No.Where it bites:
basecamp connect setup(Guided connect setup, and notices that speak to the person in the thread #794): "Work in all of your agent's projects?" defaults to Yes, but Enter answers No and setup ends with "No projects were chosen, so nothing was set up".basecamp setup: "Set up Basecamp for your coding agents?" and "Set a default project?" both behave the same way.forms.golast changed in August, so this is probably in v0.11.0 too.The prompt now opens on the default it's given. The skill-overwrite prompt defaults to No and behaves as before.
The exception is accessible mode (
TERM=dumb). There huh answers end of input with the prompt's default and can't tell it from Enter (accessibility.go:147), so a Yes default would let Ctrl+D consent. In that mode the prompt opens on No ([y/N]), and a Yes has to be typed.The tests run
Confirmthrough a smallrunConfirmFormseam and answer it via the field's own input handling: Enter, and answers typed against the default. They also check that end of input in accessible mode never gives Yes. They fail with the old code and with a prompt that isn't bound toConfirm's result.Card: https://app.basecamp.com/2914079/buckets/48699913/card_tables/cards/10356301063
Summary by cubic
Fixes
tui.Confirmso a yes/no prompt opens on the default answer, letting Enter pick it.Every prompt previously opened on No, so pressing Enter answered No in
basecamp connect setupandbasecamp setupeven when the question meant Yes. The skill-overwrite prompt defaults to No and behaves as before. Tests answer the form through arunConfirmFormseam and drive the field key by key with the key map the runForm gives every form, checking Enter picks the default, typed answers override it, and answering moves on.The exception is accessible mode (
TERM=dumb): huh answers end of input with the prompt's default and can't tell it from Enter, so a Yes default would let Ctrl+D consent. There the prompt opens on No, and a Yes must be typed. A test covers this too.bubbleteav1.3.10 is now a direct requirement; the test imports it.Written for commit 37b6db3. Summary will update on new commits.