Skip to content

Commit d582cb4

Browse files
Copilotdjeada
andauthored
Fix status tracking in hooks due to subshell variable scope (#25)
* Initial plan * Fix critical bug in _run_all.sh and add comprehensive hooks documentation Co-authored-by: djeada <[email protected]> * Fix subshell bug in last_line_empty.sh and remove_trailing_whitespaces.sh Co-authored-by: djeada <[email protected]> * Fix subshell bug in beautify_script.sh Co-authored-by: djeada <[email protected]> * Fix shebang placement in _run_all.sh Co-authored-by: djeada <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: djeada <[email protected]>
1 parent 64131a7 commit d582cb4

File tree

5 files changed

+120
-10
lines changed

5 files changed

+120
-10
lines changed

hooks/README.md

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# Hooks
2+
3+
This directory contains scripts that can be used as pre-commit hooks or CI checks to maintain code quality in the repository.
4+
5+
## Overview
6+
7+
The hooks are designed to automatically check and fix common issues in bash scripts, such as:
8+
- Removing carriage return characters (Windows line endings)
9+
- Ensuring files end with exactly one empty line
10+
- Removing trailing whitespaces
11+
- Formatting and linting bash scripts
12+
13+
## Usage
14+
15+
### Running All Hooks
16+
17+
To run all hooks in check mode (without modifying files):
18+
19+
```bash
20+
./hooks/_run_all.sh
21+
```
22+
23+
This will execute all hooks with the `--check` option on the `src` directory. It will exit with status 1 if any checks fail.
24+
25+
### Running Individual Hooks
26+
27+
Each hook can be run individually with the following syntax:
28+
29+
```bash
30+
./hooks/<hook_name>.sh [--check] <path>
31+
```
32+
33+
- `--check`: Only check if changes are needed, do not modify files
34+
- `<path>`: File or directory to process
35+
36+
**Examples:**
37+
38+
```bash
39+
# Check a single file for carriage returns
40+
./hooks/remove_carriage_return.sh --check src/my_script.sh
41+
42+
# Remove trailing whitespaces from all files in src directory
43+
./hooks/remove_trailing_whitespaces.sh src
44+
45+
# Check if all files end with exactly one empty line
46+
./hooks/last_line_empty.sh --check src
47+
```
48+
49+
## Available Hooks
50+
51+
### beautify_script.sh
52+
Formats shell scripts using Beautysh and analyzes them with ShellCheck.
53+
54+
**Requirements:**
55+
- `beautysh` - Install with `pip3 install beautysh`
56+
- `shellcheck` - Install with `apt-get install shellcheck` (Debian/Ubuntu)
57+
58+
### remove_carriage_return.sh
59+
Removes carriage return characters (`\r`) from files. This is useful for files that may have been edited on Windows systems.
60+
61+
### last_line_empty.sh
62+
Ensures that all files end with exactly one empty line. This is a common convention in many projects.
63+
64+
### remove_trailing_whitespaces.sh
65+
Removes trailing whitespace characters (spaces or tabs) from the end of lines in files.
66+
67+
## Integration
68+
69+
### CI/CD Integration
70+
71+
These hooks are automatically run in the CI pipeline (see `.github/workflows/blank.yml`). The CI will fail if any checks don't pass.
72+
73+
### Git Pre-commit Hook (Optional)
74+
75+
To run these checks before every commit, you can create a git pre-commit hook:
76+
77+
```bash
78+
cat > .git/hooks/pre-commit << 'EOF'
79+
#!/usr/bin/env bash
80+
./hooks/_run_all.sh
81+
EOF
82+
chmod +x .git/hooks/pre-commit
83+
```
84+
85+
This will automatically run all hooks before each commit. If any checks fail, the commit will be aborted.
86+
87+
## How It Works
88+
89+
The scripts in this directory are actually symbolic links to the corresponding scripts in the `src` directory. This allows the same scripts to be used both as utility scripts and as hooks.
90+
91+
The `_run_all.sh` script:
92+
1. Finds all symbolic links in the `hooks` directory (excluding files starting with `_`)
93+
2. Executes each script with the `--check` flag
94+
3. Collects the exit status of all scripts
95+
4. Exits with status 1 if any check failed
96+
97+
## Troubleshooting
98+
99+
If a hook fails in CI or locally:
100+
101+
1. Read the error message to understand which check failed
102+
2. Run the specific hook without `--check` to automatically fix the issue:
103+
```bash
104+
./hooks/<hook_name>.sh src
105+
```
106+
3. Commit the changes and try again
107+
108+
## Note
109+
110+
All hooks respect the `--check` flag. When used with `--check`, they will only report issues without modifying files. Without this flag, they will automatically fix the issues.

hooks/_run_all.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
# hooks/_run_all.sh
21
#!/usr/bin/env bash
2+
# hooks/_run_all.sh
33

44
# Ensure tput has something to work with
55
export TERM=${TERM:-dumb}
@@ -24,7 +24,8 @@ RESET=$(tput sgr0)
2424
# Process each path
2525
for path in "${paths[@]}"; do
2626
# Find all scripts (not starting with _) in 'hooks' directory
27-
find hooks -type l -name "[^_]*.sh" | while read -r script; do
27+
# Use process substitution to avoid subshell and preserve status variable
28+
while read -r script; do
2829
echo -e "\nExecuting $script"
2930

3031
# Execute the script with '--check' option
@@ -34,7 +35,7 @@ for path in "${paths[@]}"; do
3435
echo "${RED}${script} check on ${path} failed${RESET}"
3536
status=1
3637
fi
37-
done
38+
done < <(find hooks -type l -name "[^_]*.sh")
3839
done
3940

4041
# Exit with 1 if any check failed

src/beautify_script.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ process_path() {
7070
local path="$1"
7171
local checkonly="$2"
7272
if [ -d "$path" ]; then
73-
find "$path" -type f -name '*.sh' -print0 | while IFS= read -r -d '' file; do
73+
while IFS= read -r -d '' file; do
7474
format_file "$file" "$checkonly"
75-
done
75+
done < <(find "$path" -type f -name '*.sh' -print0)
7676
elif [ -f "$path" ]; then
7777
format_file "$path" "$checkonly"
7878
else

src/last_line_empty.sh

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,9 @@ process_directory() {
136136

137137
echo "Recursively processing directory: $directory"
138138
# Use find to traverse
139-
find "$directory" -type f -print0 2>/dev/null \
140-
| while IFS= read -r -d '' f; do
139+
while IFS= read -r -d '' f; do
141140
process_file "$f"
142-
done
141+
done < <(find "$directory" -type f -print0 2>/dev/null)
143142
}
144143

145144
###############################################################################

src/remove_trailing_whitespaces.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ main() {
6262
local path="$1"
6363

6464
if [ "$path" == '.' ] || [ -d "$path" ]; then
65-
find "$path" -maxdepth 10 -type f ! -name "*.tmp" ! -regex ".*/$(basename "$0")" -print0 | while IFS= read -r -d '' file; do
65+
while IFS= read -r -d '' file; do
6666
remove_trailing_whitespaces "$file"
67-
done
67+
done < <(find "$path" -maxdepth 10 -type f ! -name "*.tmp" ! -regex ".*/$(basename "$0")" -print0)
6868
elif [ -f "$path" ]; then
6969
if [ "$(basename "$path")" != "$scriptname" ]; then
7070
remove_trailing_whitespaces "$path"

0 commit comments

Comments
 (0)