Fix #74: [Bug]: Default docker status is not configured properly for display#75
Fix #74: [Bug]: Default docker status is not configured properly for display#75itniuma2026 wants to merge 3 commits intogaureshpai:mainfrom
Conversation
The Docker prompt's default value is not properly configured, causing it to display incorrectly when the user accepts the default. The default should be 'N' (No) and should be displayed/selected properly. Fixes gaureshpai#74
📝 WalkthroughWalkthroughConfirm prompt behavior in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Hi @itniuma2026!
Thanks for submitting your first Pull Request!
We’re thrilled to have your contribution and will review it shortly.
✅ CI/CD Test ResultsOverall Status: success |
|
Updated the PR description to follow the repository template structure with proper sections (Type of Change, Checklist, Testing, etc.). Code changes remain the same as reviewed. |
The confirm prompts now display '(default: Yes)' or '(default: No)' text in addition to the (Y/n) hint, making the default option much clearer. Before: ? Do you want to use TypeScript? (Y/n) After: ? Do you want to use TypeScript? (default: Yes) (Y/n) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
|
Hi @gaureshpai, I've identified and fixed the issue! The boolean prompts weren't displaying the default values clearly. They only showed What was changedUpdated the prompt display in Before vs AfterBefore: After: This makes the default option much clearer and consistent with the documentation. The fix has been pushed to this PR. Please test it and let me know if this resolves the display issue! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/prompts.js (1)
24-26: Inconsistent boolean check betweendisplayMessageandconfirmHint.Line 24 uses a truthy check (
defaultVal ? "Yes" : "No") while line 26 now correctly uses a strict boolean check (defaultVal === true). If a non-boolean truthy value were passed asdefaultVal, the prompt would display contradictory hints (e.g.,"(default: Yes) (y/N)").For consistency, apply the same strict check in
displayMessage:♻️ Proposed fix
- const displayMessage = `${message}${defaultVal !== undefined ? ` (default: ${type === "confirm" ? (defaultVal ? "Yes" : "No") : defaultVal})` : ""} `; + const displayMessage = `${message}${defaultVal !== undefined ? ` (default: ${type === "confirm" ? (defaultVal === true ? "Yes" : "No") : defaultVal})` : ""} `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/prompts.js` around lines 24 - 26, The displayMessage uses a truthy check for defaultVal while confirmHint uses a strict boolean check, causing inconsistent output; update the displayMessage construction (the template that uses `${type === "confirm" ? (defaultVal ? "Yes" : "No") : defaultVal}`) to use a strict boolean comparison like `defaultVal === true` when type === "confirm" so both displayMessage and confirmHint evaluate booleans the same way and non-boolean truthy defaults no longer produce contradictory hints.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/prompts.js`:
- Around line 24-26: The displayMessage uses a truthy check for defaultVal while
confirmHint uses a strict boolean check, causing inconsistent output; update the
displayMessage construction (the template that uses `${type === "confirm" ?
(defaultVal ? "Yes" : "No") : defaultVal}`) to use a strict boolean comparison
like `defaultVal === true` when type === "confirm" so both displayMessage and
confirmHint evaluate booleans the same way and non-boolean truthy defaults no
longer produce contradictory hints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d085171d-76ea-45ae-87ea-0aac31390bb1
📒 Files selected for processing (1)
src/lib/prompts.js
📜 Review details
🔇 Additional comments (1)
src/lib/prompts.js (1)
29-31: LGTM!Using
displayMessageensures the explicit default indicator (e.g.,"(default: No)") is shown alongside the(Y/n)or(y/N)hint, making the prompt clearer for users.
Make displayMessage use strict boolean check (defaultVal === true) to match confirmHint behavior and prevent contradictory hints when non-boolean truthy values are passed as defaultVal.
|
Perfect catch by CodeRabbit! I've fixed the boolean check inconsistency. Issue IdentifiedCodeRabbit correctly identified that
This could cause contradictory hints like Fix AppliedUpdated line 24 to use the same strict boolean comparison: // BEFORE:
const displayMessage = `${message}${defaultVal !== undefined ? ` (default: ${type === "confirm" ? (defaultVal ? "Yes" : "No") : defaultVal})` : ""} `;
// AFTER:
const displayMessage = `${message}${defaultVal !== undefined ? ` (default: ${type === "confirm" ? (defaultVal === true ? "Yes" : "No") : defaultVal})` : ""} `;Now both The fix has been pushed and should resolve the consistency concern! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/prompts.js (1)
33-35:⚠️ Potential issue | 🟠 MajorNormalize confirm default result to boolean on Enter.
On empty input, Line [34] returns raw
defaultVal. IfdefaultValis a truthy string (e.g.,"false"), the answer is not boolean and can be misinterpreted downstream as enabled.💡 Proposed fix
if (trimmed === "") { - answer = defaultVal !== undefined ? defaultVal : false; + answer = defaultVal === true; break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/prompts.js` around lines 33 - 35, The confirm prompt currently assigns answer = defaultVal (which may be a truthy string like "false") when trimmed input is empty, causing non-boolean results; update the logic in the confirm handler (the block where trimmed === "" sets answer) to coerce defaultVal to an actual boolean (e.g., answer = Boolean(defaultVal) or compare defaultVal to the string "false"/"true") so that answer is always true/false rather than a raw string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/prompts.js`:
- Around line 24-30: displayMessage currently always appends a "(default: ...)"
suffix which causes duplicate default text for confirm prompts because
confirmHint and the original message may already include it; update the
displayMessage construction to only add the "(default: ...)" suffix when
defaultVal is defined AND type !== "confirm" (or when the message does not
already contain "(default:"), so in the code that builds displayMessage change
the condition to skip adding the default for confirm prompts (referencing
displayMessage, confirmHint, ask and the type === "confirm" branch).
---
Outside diff comments:
In `@src/lib/prompts.js`:
- Around line 33-35: The confirm prompt currently assigns answer = defaultVal
(which may be a truthy string like "false") when trimmed input is empty, causing
non-boolean results; update the logic in the confirm handler (the block where
trimmed === "" sets answer) to coerce defaultVal to an actual boolean (e.g.,
answer = Boolean(defaultVal) or compare defaultVal to the string "false"/"true")
so that answer is always true/false rather than a raw string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43d36e3a-d20e-4a42-a2ec-4fb5e03093a2
📒 Files selected for processing (1)
src/lib/prompts.js
📜 Review details
🔇 Additional comments (1)
src/lib/prompts.js (1)
26-26: Strict boolean check for confirm hint is a good fix.
defaultVal === true ? "(Y/n)" : "(y/N)"correctly prevents truthy non-boolean values from showing the affirmative hint.
| const displayMessage = `${message}${defaultVal !== undefined ? ` (default: ${type === "confirm" ? (defaultVal === true ? "Yes" : "No") : defaultVal})` : ""} `; | ||
|
|
||
| const confirmHint = defaultVal ? "(Y/n)" : "(y/N)"; | ||
| const confirmHint = defaultVal === true ? "(Y/n)" : "(y/N)"; | ||
| if (type === "confirm") { | ||
| while (true) { | ||
| const input = await ask( | ||
| `${chalk.green("?")} ${chalk.bold(message)} ${chalk.dim(confirmHint)} `, | ||
| `${chalk.green("?")} ${chalk.bold(displayMessage)} ${chalk.dim(confirmHint)} `, |
There was a problem hiding this comment.
Avoid duplicate (default: ...) text in confirm prompts.
Line [24] appends (default: ...), and Line [30] now renders displayMessage. In src/create-next-quick.js Line [240]-[243], the message already contains (default: Yes), so this can render duplicate suffixes.
💡 Proposed fix
- const displayMessage = `${message}${defaultVal !== undefined ? ` (default: ${type === "confirm" ? (defaultVal === true ? "Yes" : "No") : defaultVal})` : ""} `;
+ const hasDefaultSuffix = /\(default:\s*[^)]+\)/i.test(message);
+ const displayMessage = `${message}${
+ defaultVal !== undefined && !hasDefaultSuffix
+ ? ` (default: ${type === "confirm" ? (defaultVal === true ? "Yes" : "No") : defaultVal})`
+ : ""
+ } `;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const displayMessage = `${message}${defaultVal !== undefined ? ` (default: ${type === "confirm" ? (defaultVal === true ? "Yes" : "No") : defaultVal})` : ""} `; | |
| const confirmHint = defaultVal ? "(Y/n)" : "(y/N)"; | |
| const confirmHint = defaultVal === true ? "(Y/n)" : "(y/N)"; | |
| if (type === "confirm") { | |
| while (true) { | |
| const input = await ask( | |
| `${chalk.green("?")} ${chalk.bold(message)} ${chalk.dim(confirmHint)} `, | |
| `${chalk.green("?")} ${chalk.bold(displayMessage)} ${chalk.dim(confirmHint)} `, | |
| const hasDefaultSuffix = /\(default:\s*[^)]+\)/i.test(message); | |
| const displayMessage = `${message}${ | |
| defaultVal !== undefined && !hasDefaultSuffix | |
| ? ` (default: ${type === "confirm" ? (defaultVal === true ? "Yes" : "No") : defaultVal})` | |
| : "" | |
| } `; | |
| const confirmHint = defaultVal === true ? "(Y/n)" : "(y/N)"; | |
| if (type === "confirm") { | |
| while (true) { | |
| const input = await ask( | |
| `${chalk.green("?")} ${chalk.bold(displayMessage)} ${chalk.dim(confirmHint)} `, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/prompts.js` around lines 24 - 30, displayMessage currently always
appends a "(default: ...)" suffix which causes duplicate default text for
confirm prompts because confirmHint and the original message may already include
it; update the displayMessage construction to only add the "(default: ...)"
suffix when defaultVal is defined AND type !== "confirm" (or when the message
does not already contain "(default:"), so in the code that builds displayMessage
change the condition to skip adding the default for confirm prompts (referencing
displayMessage, confirmHint, ask and the type === "confirm" branch).
gaureshpai
left a comment
gaureshpai
left a comment
There was a problem hiding this comment.
redundancy in default: Yes
Expected behavior:
If default is the value then display the option Yes
Current behavior:
Re prints default: Yes
|
I prepared a follow-up fix locally for the regression introduced by rendering Changes covered:
Validation used:
I could not push this commit directly to |
|
@Oranquelui would appreciate a PR on this @itniuma2026 any updates? |
|
I could not push directly to the contributor fork, so I opened a follow-up PR with the validated fix here: This follow-up includes the duplicate default-label fix, confirm default normalization, and regression coverage. |
|
Hi @gaureshpai, thanks for following up! @Oranquelui has already opened #78 with a more comprehensive fix that builds on the work here — it includes duplicate default-label prevention, confirm default normalization, and regression tests. I'd recommend merging #78 instead, as it covers the remaining edge cases. Happy to close this PR in favor of that one if you agree. Let me know! |
|
LGTM! Thank you for your contributions. Do consider checking out other issues to work on! |
3 participants
Description
Fixed the Docker prompt default value display issue. The confirmation hint was using a truthy check (
defaultVal) instead of strict equality (defaultVal === true), causing non-boolean truthy values like strings to incorrectly show(Y/n)instead of(y/N).Related Issue
Fixes #74
Type of Change
Testing
default: falsenow correctly display(y/N)default: truestill correctly display(Y/n)Checklist
Screenshots
N/A - This is a logic fix in the prompt hint selection.
Additional Notes
The fix changes
defaultVal ? "(Y/n)" : "(y/N)"todefaultVal === true ? "(Y/n)" : "(y/N)"insrc/lib/prompts.js(line 26). This ensures only an explicit booleantruetriggers the affirmative hint, while all other values (including truthy non-booleans) default to the negative hint.Summary by CodeRabbit
(Y/n)vs(y/N)by enforcing stricter boolean handling.