# 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:

```bash
# 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**
```bash
# 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**
```bash
# 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**
```bash
# 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:**
```bash
# 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:**
```bash
# 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**

```bash
# 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)