Skip to content

Commit da46677

Browse files
committed
Address CodeRabbit feedback: improve script robustness and formatting
- Replace 'which' with POSIX-compliant 'command -v' in test scripts - Fix markdown formatting issues in DEVELOPMENT.md (add blank lines) - Separate variable declaration and assignment in test_nvim_detection.sh - Add robust absolute path resolution using readlink and BASH_SOURCE - Enhance error handling with 'set -euo pipefail' for strict mode - Replace unreliable $0 references in bash -c contexts These changes improve portability, reliability, and follow shell scripting best practices while addressing all linting and formatting issues.
1 parent d3693c1 commit da46677

3 files changed

Lines changed: 17 additions & 8 deletions

File tree

DEVELOPMENT.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,13 @@ make test-config
188188
When running tests from within a Neovim instance (such as when using Claude Code via claude-code.nvim), the test script automatically handles the `$NVIM` environment variable which normally points to a socket file instead of the nvim executable.
189189

190190
The test script will:
191+
191192
- Use the `$NVIM` variable if it points to a valid executable file
192193
- Fall back to finding `nvim` in `$PATH` if `$NVIM` points to a socket or invalid path
193194
- Display which nvim binary is being used for transparency
194195

195196
To verify the NVIM detection logic works correctly, you can run:
197+
196198
```bash
197199
./scripts/test_nvim_detection.sh
198200
```

scripts/test.sh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ if [ -n "$NVIM" ] && [ -x "$NVIM" ] && [ ! -S "$NVIM" ]; then
1818
echo "Using NVIM from environment: $NVIM"
1919
else
2020
# Find nvim in PATH
21-
NVIM=$(which nvim)
22-
if [ -z "$NVIM" ]; then
21+
if ! NVIM=$(command -v nvim); then
2322
echo "Error: nvim not found in PATH"
2423
exit 1
2524
fi

scripts/test_nvim_detection.sh

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
#!/bin/bash
2-
set -e
2+
set -euo pipefail
33

44
# Test script to verify NVIM environment variable detection logic
55
# This script tests the fix for handling NVIM variable when running inside Neovim
66

7+
# Get the absolute directory path of this script
8+
SCRIPT_DIR="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
9+
PROJECT_DIR="$(dirname "$SCRIPT_DIR")"
10+
711
echo "Testing NVIM environment variable detection..."
812

913
# Save original NVIM value
@@ -13,7 +17,7 @@ ORIGINAL_NVIM="$NVIM"
1317
echo "Test 1: NVIM points to a socket"
1418
export NVIM="/tmp/test_socket"
1519
mkfifo "$NVIM" 2>/dev/null || true # Create a named pipe (similar to socket)
16-
if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then
20+
if timeout 5 bash -c "cd '$PROJECT_DIR' && ./scripts/test.sh" 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then
1721
echo "✓ Fallback to PATH works"
1822
else
1923
echo "✗ Fallback failed"
@@ -22,8 +26,12 @@ rm -f "$NVIM"
2226

2327
# Test 2: NVIM points to valid executable
2428
echo "Test 2: NVIM points to valid executable"
25-
export NVIM="$(which nvim)"
26-
if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Using NVIM from environment"; then
29+
if ! NVIM=$(command -v nvim); then
30+
echo "Error: nvim not found in PATH"
31+
exit 1
32+
fi
33+
export NVIM
34+
if timeout 5 bash -c "cd '$PROJECT_DIR' && ./scripts/test.sh" 2>&1 | head -10 | grep -q "Using NVIM from environment"; then
2735
echo "✓ Using provided NVIM works"
2836
else
2937
echo "✗ Using provided NVIM failed"
@@ -32,7 +40,7 @@ fi
3240
# Test 3: NVIM points to non-existent path
3341
echo "Test 3: NVIM points to non-existent path"
3442
export NVIM="/nonexistent/nvim"
35-
if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then
43+
if timeout 5 bash -c "cd '$PROJECT_DIR' && ./scripts/test.sh" 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then
3644
echo "✓ Fallback from invalid path works"
3745
else
3846
echo "✗ Fallback from invalid path failed"
@@ -41,7 +49,7 @@ fi
4149
# Test 4: NVIM is unset
4250
echo "Test 4: NVIM is unset"
4351
unset NVIM
44-
if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then
52+
if timeout 5 bash -c "cd '$PROJECT_DIR' && ./scripts/test.sh" 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then
4553
echo "✓ Unset NVIM works"
4654
else
4755
echo "✗ Unset NVIM failed"

0 commit comments

Comments
 (0)