Files
Linux-Server-Management-Too…/CODING_GUIDELINES.md
T
cschantz 57403fe715 Add color code bug prevention: cecho helper + coding guidelines
PREVENTION STRATEGY for "echo without -e" bug:

1. NEW HELPER FUNCTION - cecho()
   - Added to lib/common-functions.sh (lines 100-115)
   - Wrapper around echo -e for colored output
   - Clear documentation with examples
   - Usage: cecho "${BOLD}Text${NC}" instead of echo -e

2. COMPREHENSIVE CODING GUIDELINES
   - Created CODING_GUIDELINES.md
   - Documents the echo -e color bug with examples
   - Prevention rules and quick reference table
   - Search command to find potential issues
   - Pre-commit checklist for developers
   - Performance guidelines (subprocess elimination)

3. DOCUMENTATION INCLUDES:
   - Why the bug happens (escape sequences not interpreted)
   - How to identify it (grep pattern)
   - How to fix it (echo -e or cecho)
   - When to use each approach
   - Historical context (commit 7053b3b)

BENEFITS:
- Future developers can reference guidelines
- cecho() provides cleaner, safer API
- Search pattern helps audit existing code
- Reduces recurring "This happens a lot" issues

USER FEEDBACK ADDRESSED:
User: "This happens a lot with you. is there a way for us to avoid this in the future?"
Answer: Yes - cecho() helper + guidelines document + search pattern

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-02 17:14:19 -05:00

5.0 KiB

Server Toolkit Coding Guidelines

Color Code Usage - CRITICAL

The Problem

Using echo without the -e flag causes ANSI escape sequences to display literally instead of being interpreted:

# WRONG - Shows literal: \033[1m1\033[0m
echo "  ${BOLD}1${NC} - Menu option"

# RIGHT - Shows: 1 (bold and colored)
echo -e "  ${BOLD}1${NC} - Menu option"

Prevention Rules

Rule 1: Always use echo -e when string contains color variables

# WRONG:
echo "  ${BOLD}Option${NC}"
echo "Status: ${GREEN}OK${NC}"

# RIGHT:
echo -e "  ${BOLD}Option${NC}"
echo -e "Status: ${GREEN}OK${NC}"

Rule 2: Use helper functions from common-functions.sh

# Source the library
source /root/server-toolkit/lib/common-functions.sh

# Use built-in print functions (already use echo -e)
print_success "Operation completed"
print_error "Something went wrong"
print_info "Information message"

# For custom colored output, use cecho()
cecho "Custom ${RED}colored${NC} text"

Rule 3: No -e flag needed when NOT using quotes

# This is OK (no quotes around the whole thing)
echo ${BOLD}Title${NC}

# But this needs -e (quotes around everything)
echo -e "${BOLD}Title${NC}"

Quick Reference

Scenario Command
Menu option with colors echo -e " ${BOLD}1${NC} - Option"
Success message print_success "Done" or echo -e "${GREEN}✓${NC} Done"
Error message print_error "Failed" or echo -e "${RED}✗${NC} Failed"
Custom colored text cecho "Text ${RED}red${NC} normal"
Plain text (no colors) echo "Plain text" (no -e needed)

How to Find and Fix

Search for potential issues:

# Find echo statements with color variables that might be missing -e
grep -n 'echo ".*\${\(BOLD\|.*_COLOR\|NC\|RED\|GREEN\|YELLOW\|BLUE\)' your_script.sh

Common patterns to fix:

# BEFORE:
echo "  ${BOLD}1${NC} - Enable Feature"
echo "Status: ${GREEN}Active${NC}"
echo "${RED}[ERROR]${NC} Failed"

# AFTER:
echo -e "  ${BOLD}1${NC} - Enable Feature"
echo -e "Status: ${GREEN}Active${NC}"
echo -e "${RED}[ERROR]${NC} Failed"

# OR use helper:
cecho "  ${BOLD}1${NC} - Enable Feature"
cecho "Status: ${GREEN}Active${NC}"
print_error "Failed"

Why This Matters

Impact of the bug:

  • Menus show escape codes instead of colors: \033[1m1\033[0m
  • Makes interface look broken and unprofessional
  • Reduces readability and user experience
  • Hard to debug because it "looks right" in the code

Historical issues:

  • Security hardening menu (fixed: commit 7053b3b)
  • Various other menus and status displays
  • User feedback: "This happens a lot with you"

Pre-commit Checklist

Before committing code with color output:

  • All echo statements with ${COLOR} variables use -e flag
  • Or use cecho() helper function instead
  • Or use print_*() functions from common-functions.sh
  • Test output in terminal to verify colors render correctly
  • Run: bash -n script.sh to check syntax
  • Run: shellcheck script.sh if available

Performance Guidelines

Avoid Subprocesses in Loops

Rule: Use bash built-ins instead of spawning processes

# WRONG - Spawns subprocess (slow)
url_lower=$(echo "$url" | tr '[:upper:]' '[:lower:]')
url_lower=$(echo "$url" | tr 'A-Z' 'a-z')

# RIGHT - Bash built-in (fast)
url_lower="${url,,}"

# WRONG - Multiple subprocesses
hostname=$(hostname)
for item in $list; do
    check_hostname "$hostname"  # Calls hostname 1000s of times
done

# RIGHT - Cache the result
CACHED_HOSTNAME="${HOSTNAME:-$(hostname)}"
for item in $list; do
    check_hostname "$CACHED_HOSTNAME"
done

Impact: On high-traffic servers (1000+ req/sec), subprocess elimination prevents tens of thousands of unnecessary forks per second.

Performance Quick Reference

Operation Slow (subprocess) Fast (built-in)
Lowercase $(echo "$var" | tr '[:upper:]' '[:lower:]') ${var,,}
Uppercase $(echo "$var" | tr '[:lower:]' '[:upper:]') ${var^^}
Substring $(echo "$var" | cut -c1-5) ${var:0:5}
Replace $(echo "$var" | sed 's/old/new/') ${var//old/new}
Length $(echo "$var" | wc -c) ${#var}

Additional Guidelines

Error Handling

  • Always check return codes for critical operations
  • Use set -euo pipefail for strict error handling in new scripts
  • Provide meaningful error messages with context

Function Documentation

  • Add comment block above complex functions
  • Document parameters, return values, and side effects
  • Include usage examples for non-obvious functions

Variable Naming

  • Use descriptive names: user_count not uc
  • Constants in UPPER_CASE: MAX_RETRIES=3
  • Local variables in lower_case: local temp_file="/tmp/data"
  • Global exports in UPPER_CASE: export LOG_DIR="/var/log"

Testing

  • Test with both light and dark terminal backgrounds
  • Test with TOOLKIT_NO_COLOR=1 for monochrome output
  • Verify output doesn't overflow on 24-line terminals (standard)
  • Test with realistic data volumes (not just 1-2 entries)