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>
This commit is contained in:
cschantz
@@ -0,0 +1,172 @@
|
||||
# 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)
|
||||
Reference in New Issue
Block a user