From 19e565f1ed62c38fca70d94ba997561be7e4fd48 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 25 Jul 2026 06:55:38 +0200 Subject: [PATCH] docs: stop instructing shfmt -w, and scope make sa correctly Eight places told contributors to run `shfmt -w .`. shfmt disagrees with .editorconfig on this repo two ways -- indent_size = unset is read as tabs for tests/acceptance/**, and `-continued strings get collapsed past the 120-char limit -- so following that instruction reformats 146 tracked files and breaks make lint. make lint is the formatting authority; record how to check for new drift instead. make sa built its file list with `find .`, which descends into .claude/worktrees/ (linting the whole tree once per linked worktree) and whose *.sh glob never covered the extensionless entrypoint, bin/pre-commit or bin/create-pr. Take the list from git instead, keeping find as the non-git fallback: 274 files now, including the three that were never checked. --- .claude/CLAUDE.md | 33 +++++++++++++++++++++++---- .claude/rules/bash-style.md | 4 +++- .claude/rules/tdd-workflow.md | 5 ++-- .claude/skills/add-assertion/SKILL.md | 2 +- .claude/skills/gh-issue/SKILL.md | 2 +- .claude/skills/pre-release/SKILL.md | 2 +- .claude/skills/review/SKILL.md | 2 +- .claude/skills/tdd-cycle/SKILL.md | 2 +- Makefile | 12 +++++++++- 9 files changed, 49 insertions(+), 15 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 2b44499f..26912109 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -27,11 +27,14 @@ See `.claude/rules/bash-style.md` for complete compatibility guide (auto-loaded Every change must pass: ```bash make sa # ShellCheck static analysis -make lint # EditorConfig linting +make lint # EditorConfig linting (the formatting authority — 2-space indent) ./bashunit tests/ # All tests passing -shfmt -w . # Code formatting ``` +**Never run `shfmt -w`.** shfmt and `.editorconfig` disagree on this repo: 146 +tracked files are already shfmt-dirty on a clean `main`, and satisfying shfmt +breaks `make lint`. See "Formatting" below. + ## Architecture ``` @@ -61,8 +64,28 @@ bashunit/ ./bashunit --parallel tests/ # Parallel execution ./bashunit tests/unit/ # Run unit tests only make sa # ShellCheck static analysis -make lint # EditorConfig checker -shfmt -w . # Format all shell files +make lint # EditorConfig checker (formatting authority) +``` + +## Formatting + +Match the surrounding 2-space style by hand and verify with `make lint`. There is +no `shfmt` target, and **`shfmt -w .` must not be run** — it fights `.editorconfig` +two ways: + +- `.editorconfig` sets `indent_size = unset` for `tests/acceptance/**.sh` and + `src/console_header.sh`; shfmt reads `unset` as "use my default", which is tabs, + while `[*]` sets `indent_style = space`. Any new file under `tests/acceptance/` + therefore shows up shfmt-dirty — that is expected, not a defect. +- shfmt collapses `\`-continued strings onto one line, which would push + `src/state.sh`'s record writer past `max_line_length = 120`. + +To check you introduced no new drift, compare the dirty-file *list* against main +rather than requiring it to be empty: + +```bash +git ls-files -z '*.sh' bashunit | xargs -0 shfmt -d 2>/dev/null \ + | grep '^--- ' | sed 's|--- ||; s|\.orig.*||' | sort ``` ## Test Patterns @@ -137,7 +160,7 @@ Rules auto-load based on file paths being edited (via `paths:` frontmatter in ea - All tests green for the **right reason** - `make sa` passes (ShellCheck) - `make lint` passes (EditorConfig) -- Code formatted (`shfmt -w .`) +- Code formatted to 2-space indent by hand (verified by `make lint`, never `shfmt -w`) - Bash 3.0+ compatible - Parallel tests passing (`./bashunit --parallel tests/`) - CHANGELOG.md updated (if user-facing changes) diff --git a/.claude/rules/bash-style.md b/.claude/rules/bash-style.md index 2976e4e3..fdd59c76 100644 --- a/.claude/rules/bash-style.md +++ b/.claude/rules/bash-style.md @@ -20,7 +20,9 @@ bashunit must work on **Bash 3.0+** (macOS default). These features are **prohib ## Coding Conventions -- **2 spaces** indent, no tabs — enforced by `shfmt -w .` +- **2 spaces** indent, no tabs — enforced by `make lint` (EditorConfig). Do **not** + run `shfmt -w`: it wants tabs for `tests/acceptance/**` and collapses + `\`-continued strings past the 120-char limit, breaking `make lint` - **120 chars** max line length (soft) - Follow [Google Shell Style Guide](https://google.github.io/styleguide/shellguide.html) - Always quote variables unless explicit word splitting is needed diff --git a/.claude/rules/tdd-workflow.md b/.claude/rules/tdd-workflow.md index e12358e1..73c916ea 100644 --- a/.claude/rules/tdd-workflow.md +++ b/.claude/rules/tdd-workflow.md @@ -59,15 +59,14 @@ Test: (none yet) 1. Improve readability, naming, extract duplication — **no behavior changes** 2. Run tests after each change -3. Run quality checks: `make sa && make lint && shfmt -w .` +3. Run quality checks: `make sa && make lint` (never `shfmt -w` — it breaks `make lint`) ## Quality Gate (Before Commit) ```bash ./bashunit tests/ # All tests ./bashunit --parallel tests/ # Parallel (isolation check) -make sa && make lint # Static analysis + linting -shfmt -w . # Formatting +make sa && make lint # Static analysis + linting (lint is the format gate) ``` ## Definition of Done diff --git a/.claude/skills/add-assertion/SKILL.md b/.claude/skills/add-assertion/SKILL.md index 17f91b84..4bf4355d 100644 --- a/.claude/skills/add-assertion/SKILL.md +++ b/.claude/skills/add-assertion/SKILL.md @@ -41,7 +41,7 @@ For each test in inventory, follow RED -> GREEN -> REFACTOR: - Source new file in `src/bashunit.sh` if created - `export -f` the assertion function - Run full test suite: `./bashunit tests/` -- Quality checks: `make sa && make lint && shfmt -w .` +- Quality checks: `make sa && make lint` ### 5. Documentation diff --git a/.claude/skills/gh-issue/SKILL.md b/.claude/skills/gh-issue/SKILL.md index 84fa6ce7..48d91e85 100644 --- a/.claude/skills/gh-issue/SKILL.md +++ b/.claude/skills/gh-issue/SKILL.md @@ -83,7 +83,7 @@ Fetch a GitHub issue, create branch, implement following TDD, and open a PR. 12. **Quality checks** after each refactor: ```bash - make sa && make lint && shfmt -w . + make sa && make lint ``` ### Phase 4: Ship diff --git a/.claude/skills/pre-release/SKILL.md b/.claude/skills/pre-release/SKILL.md index d10a9735..a425f072 100644 --- a/.claude/skills/pre-release/SKILL.md +++ b/.claude/skills/pre-release/SKILL.md @@ -31,7 +31,7 @@ All must pass. Run 3-5 times to catch flaky tests. ```bash make sa # ShellCheck — zero warnings make lint # EditorConfig — clean -shfmt -l . # Check formatting (don't modify) +# NB: no shfmt gate — it conflicts with .editorconfig here; make lint is the authority ``` ### 4. Documentation diff --git a/.claude/skills/review/SKILL.md b/.claude/skills/review/SKILL.md index 5a7fbf26..9a77db4c 100644 --- a/.claude/skills/review/SKILL.md +++ b/.claude/skills/review/SKILL.md @@ -36,7 +36,7 @@ Review a GitHub PR. When no number is given, review the PR for the current branc - Bash 3.0+ compatibility (no `declare -A`, `[[ ]]`, `${var,,}`, negative indexing, `&>>`) - Tests exist and follow TDD; both success and failure paths covered - Naming, namespacing (`bashunit::*` / `_private`), and dynamic-scope safety - - ShellCheck cleanliness and `shfmt` formatting + - ShellCheck cleanliness and 2-space indentation (`make lint`, not `shfmt`) - CHANGELOG.md updated for user-facing changes 4. **Report findings** grouped by severity (blocker / suggestion / nit), each as `path:line — problem. fix.`. No praise, no scope creep. State plainly if the PR looks good. diff --git a/.claude/skills/tdd-cycle/SKILL.md b/.claude/skills/tdd-cycle/SKILL.md index 4743e96e..8f0a4b65 100644 --- a/.claude/skills/tdd-cycle/SKILL.md +++ b/.claude/skills/tdd-cycle/SKILL.md @@ -33,7 +33,7 @@ Check for `.tasks/YYYY-MM-DD-*.md`. If missing, create one before proceeding. 1. Improve readability, naming, extract duplication — no behavior changes 2. Run tests after each change -3. Quality checks: `make sa && make lint && shfmt -w .` +3. Quality checks: `make sa && make lint` 4. Full suite: `./bashunit tests/` 5. Update task file with refactoring notes diff --git a/Makefile b/Makefile index 9896844f..a7e48bcf 100644 --- a/Makefile +++ b/Makefile @@ -94,11 +94,21 @@ test/parallel: $(TEST_SCRIPTS) # SHELLCHECK_OPTS for local/CI parity. One file per invocation (like CI's # action) because shellcheck 0.11.0 can crash on multi-file batches with -x; # -P 4 keeps the wall time reasonable. +# +# The file list comes from git, not `find`, for two reasons: `find .` descends +# into .claude/worktrees/ (linked worktrees hold whole copies of the tree, so +# the target lints the repo N+1 times and appears to hang), and its "*.sh" glob +# never sees the extensionless entrypoint or the bin/ scripts. `find` remains +# the fallback outside a git checkout. sa: ifndef STATIC_ANALYSIS_CHECKER @printf "\e[1m\e[31m%s\e[0m\n" "Shellcheck not installed: Static analysis not performed!" && exit 1 else - @find . -name "*.sh" -not -path "./local/*" -print0 \ + @{ if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then \ + git ls-files -z "*.sh" bashunit bin/pre-commit bin/create-pr; \ + else \ + find . -name "*.sh" -not -path "./local/*" -not -path "./.claude/worktrees/*" -print0; \ + fi; } \ | xargs -0 -n 1 -P 4 shellcheck -xC -e SC1091 -e SC2155 -e SC2016 \ && printf "\e[1m\e[32m%s\e[0m\n" "ShellCheck: OK!" endif