Skip to content

Commit d84c83e

Browse files
author
Test User
committed
fix(enable): address code review feedback
Fixes from PR #124 review: 1. sed -i portability (ralph_enable.sh:456) - Use portable sed + mv pattern instead of GNU-only sed -i 2. sed regex portability (lib/task_sources.sh) - Replace \s with POSIX [[:space:]] character class - Add sed -E flag for extended regex 3. jq availability check (ralph_enable_ci.sh:177) - Add check for jq when --json flag is used 4. Unused filter parameter (lib/task_sources.sh:44) - Pass filter to bd list --filter command 5. Word-splitting in select_multiple (ralph_enable.sh:322) - Return comma-separated indices instead of space-separated text - Update caller to parse indices correctly 6. Missing || true for check_existing_ralph (ralph_enable.sh:185) - Prevent set -e from exiting on non-zero return 7. select_multiple stdout corruption (lib/wizard_utils.sh) - Redirect interactive output to stderr - Only final result goes to stdout 8. Color variables not exported (lib/wizard_utils.sh:12) - Export WIZARD_* color variables for subshells 9. select_option infinite loop (lib/wizard_utils.sh:179) - Add guard for empty options array
1 parent 1c16762 commit d84c83e

File tree

4 files changed

+63
-50
lines changed

4 files changed

+63
-50
lines changed

‎lib/task_sources.sh‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ fetch_beads_tasks() {
4949
return 1
5050
fi
5151

52-
# Try to get tasks as JSON
52+
# Try to get tasks as JSON (pass filter if provided)
5353
local json_output
54-
if json_output=$(bd list --json 2>/dev/null); then
54+
if json_output=$(bd list --json --filter "$filter" 2>/dev/null); then
5555
# Parse JSON and format as markdown tasks
5656
if command -v jq &>/dev/null; then
5757
tasks=$(echo "$json_output" | jq -r '
@@ -260,20 +260,20 @@ extract_prd_tasks() {
260260
261261
# Look for existing checkbox items
262262
local checkbox_tasks
263-
checkbox_tasks=$(grep -E '^\s*[-*]\s*\[\s*[xX ]?\s*\]' "$prd_file" 2>/dev/null)
263+
checkbox_tasks=$(grep -E '^[[:space:]]*[-*][[:space:]]*\[[[:space:]]*[xX ]?[[:space:]]*\]' "$prd_file" 2>/dev/null)
264264
if [[ -n "$checkbox_tasks" ]]; then
265265
# Normalize to unchecked format
266266
tasks=$(echo "$checkbox_tasks" | sed 's/\[x\]/[ ]/gi; s/\[X\]/[ ]/g')
267267
fi
268268
269269
# Look for numbered list items that look like tasks
270270
local numbered_tasks
271-
numbered_tasks=$(grep -E '^\s*[0-9]+\.\s+' "$prd_file" 2>/dev/null | head -20)
271+
numbered_tasks=$(grep -E '^[[:space:]]*[0-9]+\.[[:space:]]+' "$prd_file" 2>/dev/null | head -20)
272272
if [[ -n "$numbered_tasks" ]]; then
273273
while IFS= read -r line; do
274274
# Convert numbered item to checkbox
275275
local task_text
276-
task_text=$(echo "$line" | sed 's/^\s*[0-9]*\.\s*//')
276+
task_text=$(echo "$line" | sed -E 's/^[[:space:]]*[0-9]*\.[[:space:]]*//')
277277
if [[ -n "$task_text" ]]; then
278278
tasks="${tasks}
279279
- [ ] ${task_text}"
@@ -283,12 +283,12 @@ extract_prd_tasks() {
283283
284284
# Look for headings that might be task sections
285285
local headings
286-
headings=$(grep -E '^#{1,3}\s+(TODO|Tasks|Requirements|Features|Backlog|Sprint)' "$prd_file" 2>/dev/null)
286+
headings=$(grep -E '^#{1,3}[[:space:]]+(TODO|Tasks|Requirements|Features|Backlog|Sprint)' "$prd_file" 2>/dev/null)
287287
if [[ -n "$headings" ]]; then
288288
# Extract content after these headings as potential tasks
289289
while IFS= read -r heading; do
290290
local section_name
291-
section_name=$(echo "$heading" | sed 's/^#*\s*//')
291+
section_name=$(echo "$heading" | sed -E 's/^#*[[:space:]]*//')
292292
# This is informational - actual task extraction would need more context
293293
done <<< "$headings"
294294
fi
@@ -366,24 +366,24 @@ normalize_tasks() {
366366
[[ -z "$line" ]] && continue
367367
368368
# Already in checkbox format
369-
if echo "$line" | grep -qE '^\s*-\s*\[\s*[xX ]?\s*\]'; then
369+
if echo "$line" | grep -qE '^[[:space:]]*-[[:space:]]*\[[[:space:]]*[xX ]?[[:space:]]*\]'; then
370370
# Normalize the checkbox
371371
echo "$line" | sed 's/\[x\]/[ ]/gi; s/\[X\]/[ ]/g'
372372
continue
373373
fi
374374
375375
# Bullet point without checkbox
376-
if echo "$line" | grep -qE '^\s*[-*]\s+'; then
376+
if echo "$line" | grep -qE '^[[:space:]]*[-*][[:space:]]+'; then
377377
local text
378-
text=$(echo "$line" | sed 's/^\s*[-*]\s*//')
378+
text=$(echo "$line" | sed -E 's/^[[:space:]]*[-*][[:space:]]*//')
379379
echo "- [ ] $text"
380380
continue
381381
fi
382382
383383
# Numbered item
384-
if echo "$line" | grep -qE '^\s*[0-9]+\.?\s+'; then
384+
if echo "$line" | grep -qE '^[[:space:]]*[0-9]+\.?[[:space:]]+'; then
385385
local text
386-
text=$(echo "$line" | sed 's/^\s*[0-9]*\.?\s*//')
386+
text=$(echo "$line" | sed -E 's/^[[:space:]]*[0-9]*\.?[[:space:]]*//')
387387
echo "- [ ] $text"
388388
continue
389389
fi

‎lib/wizard_utils.sh‎

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
# wizard_utils.sh - Interactive prompt utilities for Ralph enable wizard
44
# Provides consistent, user-friendly prompts for configuration
55

6-
# Colors
7-
WIZARD_CYAN='\033[0;36m'
8-
WIZARD_GREEN='\033[0;32m'
9-
WIZARD_YELLOW='\033[1;33m'
10-
WIZARD_RED='\033[0;31m'
11-
WIZARD_BOLD='\033[1m'
12-
WIZARD_NC='\033[0m'
6+
# Colors (exported for subshells)
7+
export WIZARD_CYAN='\033[0;36m'
8+
export WIZARD_GREEN='\033[0;32m'
9+
export WIZARD_YELLOW='\033[1;33m'
10+
export WIZARD_RED='\033[0;31m'
11+
export WIZARD_BOLD='\033[1m'
12+
export WIZARD_NC='\033[0m'
1313

1414
# =============================================================================
1515
# BASIC PROMPTS
@@ -178,6 +178,12 @@ select_option() {
178178
local options=("$@")
179179
local num_options=${#options[@]}
180180

181+
# Guard against empty options array
182+
if [[ $num_options -eq 0 ]]; then
183+
echo ""
184+
return 1
185+
fi
186+
181187
echo -e "\n${WIZARD_BOLD}${prompt}${WIZARD_NC}"
182188
echo ""
183189

@@ -214,11 +220,16 @@ select_option() {
214220
# $@ (options) - Remaining arguments are the options
215221
#
216222
# Outputs:
217-
# Echoes space-separated list of selected options
223+
# Echoes comma-separated list of selected indices (0-based)
224+
# Returns empty string if nothing selected
218225
#
219226
# Example:
220227
# selected=$(select_multiple "Select task sources" "beads" "github" "prd")
221-
# echo "Selected: $selected"
228+
# # If user selects first and third: selected="0,2"
229+
# IFS=',' read -ra indices <<< "$selected"
230+
# for idx in "${indices[@]}"; do
231+
# echo "Selected: ${options[$idx]}"
232+
# done
222233
#
223234
select_multiple() {
224235
local prompt=$1
@@ -232,10 +243,10 @@ select_multiple() {
232243
selected[$i]=0
233244
done
234245

235-
# Display instructions
236-
echo -e "\n${WIZARD_BOLD}${prompt}${WIZARD_NC}"
237-
echo -e "${WIZARD_CYAN}(Enter numbers to toggle, press Enter when done)${WIZARD_NC}"
238-
echo ""
246+
# Display instructions (redirect to stderr to avoid corrupting return value)
247+
echo -e "\n${WIZARD_BOLD}${prompt}${WIZARD_NC}" >&2
248+
echo -e "${WIZARD_CYAN}(Enter numbers to toggle, press Enter when done)${WIZARD_NC}" >&2
249+
echo "" >&2
239250

240251
while true; do
241252
# Display options with checkboxes
@@ -245,12 +256,12 @@ select_multiple() {
245256
if [[ "${selected[$((i - 1))]}" == "1" ]]; then
246257
checkbox="[${WIZARD_GREEN}x${WIZARD_NC}]"
247258
fi
248-
echo -e " ${WIZARD_CYAN}${i})${WIZARD_NC} ${checkbox} ${opt}"
249-
((i++))
259+
echo -e " ${WIZARD_CYAN}${i})${WIZARD_NC} ${checkbox} ${opt}" >&2
260+
((i++)) || true
250261
done
251262

252-
echo ""
253-
echo -en "Toggle [1-${num_options}] or Enter to confirm: "
263+
echo "" >&2
264+
echo -en "Toggle [1-${num_options}] or Enter to confirm: " >&2
254265
read -r response
255266

256267
# Empty input = done
@@ -270,24 +281,24 @@ select_multiple() {
270281
selected[$idx]=0
271282
fi
272283
else
273-
echo -e "${WIZARD_YELLOW}Please enter a number between 1 and ${num_options}${WIZARD_NC}"
284+
echo -e "${WIZARD_YELLOW}Please enter a number between 1 and ${num_options}${WIZARD_NC}" >&2
274285
fi
275286

276287
# Clear previous display (move cursor up)
277288
# Number of lines to clear: options + 2 (prompt line + input line)
278289
for ((j = 0; j < num_options + 2; j++)); do
279-
echo -en "\033[A\033[K"
290+
echo -en "\033[A\033[K" >&2
280291
done
281292
done
282293

283-
# Build result string
294+
# Build result string (comma-separated indices)
284295
local result=""
285296
for ((i = 0; i < num_options; i++)); do
286297
if [[ "${selected[$i]}" == "1" ]]; then
287298
if [[ -n "$result" ]]; then
288-
result="$result ${options[$i]}"
299+
result="$result,$i"
289300
else
290-
result="${options[$i]}"
301+
result="$i"
291302
fi
292303
fi
293304
done

‎ralph_enable.sh‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,8 @@ phase_environment_detection() {
181181
echo "Analyzing your project..."
182182
echo ""
183183

184-
# Check for existing Ralph setup
185-
check_existing_ralph
184+
# Check for existing Ralph setup (use || true to prevent set -e from exiting)
185+
check_existing_ralph || true
186186
case "$RALPH_STATE" in
187187
"complete")
188188
print_detection_result "Ralph status" "Already enabled" "true"
@@ -314,21 +314,19 @@ phase_task_source_selection() {
314314
echo "Where would you like to import tasks from?"
315315
echo ""
316316

317-
local selected
318-
selected=$(select_multiple "Select task sources" "${options[@]}")
317+
local selected_indices
318+
selected_indices=$(select_multiple "Select task sources" "${options[@]}")
319319

320-
# Parse selected options
320+
# Parse selected indices (comma-separated)
321321
SELECTED_SOURCES=""
322-
for opt in $selected; do
323-
# Map option text back to key
324-
for i in "${!options[@]}"; do
325-
if [[ "${options[$i]}" == "$opt" ]]; then
326-
if [[ "${option_keys[$i]}" != "none" ]]; then
327-
SELECTED_SOURCES="${SELECTED_SOURCES:+$SELECTED_SOURCES }${option_keys[$i]}"
328-
fi
322+
if [[ -n "$selected_indices" ]]; then
323+
IFS=',' read -ra indices <<< "$selected_indices"
324+
for idx in "${indices[@]}"; do
325+
if [[ "${option_keys[$idx]}" != "none" ]]; then
326+
SELECTED_SOURCES="${SELECTED_SOURCES:+$SELECTED_SOURCES }${option_keys[$idx]}"
329327
fi
330328
done
331-
done
329+
fi
332330
else
333331
SELECTED_SOURCES=""
334332
fi
@@ -450,14 +448,14 @@ phase_file_generation() {
450448
exit $ENABLE_ERROR
451449
fi
452450

453-
# Update .ralphrc with specific settings
451+
# Update .ralphrc with specific settings (portable sed -i for macOS/Linux)
454452
if [[ -f ".ralphrc" ]]; then
455453
# Update max calls
456-
sed -i "s/MAX_CALLS_PER_HOUR=.*/MAX_CALLS_PER_HOUR=$CONFIG_MAX_CALLS/" .ralphrc 2>/dev/null || true
454+
sed "s/MAX_CALLS_PER_HOUR=.*/MAX_CALLS_PER_HOUR=$CONFIG_MAX_CALLS/" .ralphrc > .ralphrc.tmp && mv .ralphrc.tmp .ralphrc
457455

458456
# Update GitHub label if set
459457
if [[ -n "$CONFIG_GITHUB_LABEL" ]]; then
460-
sed -i "s/GITHUB_TASK_LABEL=.*/GITHUB_TASK_LABEL=\"$CONFIG_GITHUB_LABEL\"/" .ralphrc 2>/dev/null || true
458+
sed "s/GITHUB_TASK_LABEL=.*/GITHUB_TASK_LABEL=\"$CONFIG_GITHUB_LABEL\"/" .ralphrc > .ralphrc.tmp && mv .ralphrc.tmp .ralphrc
461459
fi
462460
fi
463461

‎ralph_enable_ci.sh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ parse_arguments() {
172172
shift
173173
;;
174174
--json)
175+
if ! command -v jq &>/dev/null; then
176+
echo "Error: --json requires jq to be installed" >&2
177+
exit 1
178+
fi
175179
OUTPUT_JSON=true
176180
shift
177181
;;

0 commit comments

Comments
 (0)