-
Notifications
You must be signed in to change notification settings - Fork 406
fix(wizard): redirect prompts to stderr for clean command substitution #130
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
Conversation
When wizard prompt functions (prompt_text, prompt_number, select_option, select_with_default) were used with command substitution, ANSI-colored prompts were captured along with user responses, corrupting .ralphrc values like: PROJECT_NAME="[0;36mProject name[0m [value]: value" Fix: - Redirect all display output (colored prompts, validation messages) to stderr using >&2, matching the pattern already used by select_multiple() - Keep only the actual response/result on stdout for command substitution Changes: - lib/wizard_utils.sh: Add >&2 to prompt_text (lines 84-87), prompt_number (lines 118-121, 131-151), confirm (lines 44, 60), select_option (lines 190-215), select_with_default (lines 330-364) - tests/unit/test_wizard_utils.bats: Add 20 new tests for stdout/stderr separation and clean command substitution results - CLAUDE.md: Update test count to 420 Test count: 420 (up from 396)
WalkthroughThe changes update the wizard utilities library to redirect all user-facing prompts and error messages to stderr while preserving stdout for command substitution capture. A comprehensive Bash/Bats unit test suite is added to validate this behavior alongside input handling, validation, and return codes. Test metadata documentation is updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Redirect wizard prompt outputs to stderr in
|
Code ReviewThis PR correctly addresses the issue of ANSI codes corrupting values by redirecting prompt display to stderr. The implementation is sound and follows bash best practices. ✅ Strengths
📝 Minor Suggestions
✅ VerificationAll 20 tests properly verify:
This PR is ready to merge. |
|
The review has been posted to PR #130. The PR correctly fixes the ANSI code capture issue by redirecting prompt displays to stderr, with comprehensive test coverage. Ready to merge. |
Code Review: Wizard Prompt stdout/stderr Separation FixThis is an excellent bug fix that addresses a critical issue with ANSI escape codes being captured in command substitution, corrupting configuration values in .ralphrc. The implementation is clean, well-tested, and follows bash best practices. Strengths1. Root Cause Analysis 2. Consistent stderr Redirection Pattern
3. Comprehensive Test Coverage
4. Test Quality
5. No Functional Changes Code Quality Observations1. Edge Case Handling
2. Consistent Implementation 3. Documentation Alignment Security and Best PracticesNo Security Concerns
Follows Bash Best Practices
RecommendationApprove - Production Ready This PR is production-ready and should be merged. The fix is:
Test Coverage Impact:
Excellent work on identifying and fixing this subtle but critical bug! The stdout/stderr separation is textbook bash scripting best practice. |

Summary
prompt_text,prompt_number,select_option,select_with_default,confirm) that were writing colored prompts to stdoutralph_enable.sh, ANSI codes were captured along with user input, corrupting.ralphrcvalues>&2) while keeping only the response on stdoutProblem
Changes
lib/wizard_utils.sh: Add>&2to all display output in 5 functionstests/unit/test_wizard_utils.bats: New test file with 20 tests for stdout/stderr separationCLAUDE.md: Update test count to 420Test plan
bats tests/unit/test_wizard_utils.bats- 20 tests passnpm test- all 420 tests passralph-enableinteractively, verify.ralphrchas clean valuesSummary by CodeRabbit
Release Notes
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.