fix: preserve partial staging in formatting hooks

AGENTS.md

@@ -135,8 +135,8 @@ cmake --build cmake-build-relwithdebinfo --target translate  # CLI: file to HTML
 ## Conventions
 
 - Formatting: clang-format per `.clang-format`, clang-tidy per `.clang-tidy`.
-  Run `scripts/format`, or install the hook with `scripts/setup`. CI enforces
-  both.
+  Run `scripts/format`; the hook installed by `scripts/setup` formats only the
+  staged content and never stages other changes. CI enforces both.
 - C++23 with three ceilings. Check a new facility against all three by building
   an object file, because `-fsyntax-only` misses codegen bugs.
   - The standard library is capped by emsdk 3.1.73 (libc++ 18.1). Available:

scripts/format

@@ -1,26 +1,14 @@
 #!/usr/bin/env bash
+set -euo pipefail
 
+cd "$(dirname "${BASH_SOURCE[0]}")/.."
 CLANG_FORMAT=${CLANG_FORMAT:-clang-format}
-
-echo "Running clang-format on all .hpp/.cpp and .h/.mm files in the project"
-echo "Using clang-format: ${CLANG_FORMAT}"
-
-${CLANG_FORMAT} --version
-
-function format_file() {
-    echo "Formatting $1"
-    ${CLANG_FORMAT} --style=file -i $1
-}
-function format_folder() {
-  export -f format_file
-  export CLANG_FORMAT
-  find $1 -type f \( -iname \*.hpp -o -iname \*.cpp -o -iname \*.h -o -iname \*.mm \) -exec bash -c 'format_file "${1}"' -- {} \;
-}
-
-format_folder src
-format_folder cli
-format_folder test/src
-format_folder python/src
-format_folder jni/src
-format_folder apple/src
-format_folder apple/include
+"$CLANG_FORMAT" --version
+
+find src cli test/src python/src jni/src apple/src apple/include wasm/src \
+  -type f \( -iname '*.hpp' -o -iname '*.cpp' -o -iname '*.cc' \
+             -o -iname '*.h' -o -iname '*.mm' \) -print0 |
+  while IFS= read -r -d '' file; do
+    echo "Formatting $file"
+    "$CLANG_FORMAT" --style=file -i "$file"
+  done

scripts/git_hooks/pre-commit/0_format

@@ -1,25 +1,35 @@
 #!/usr/bin/env bash
-{
-format_file() {
-  local file="${1}"
-  if [ -f $file ]; then
-    clang-format -i -style=file ${1}
-  fi
-}
+set -euo pipefail
 
-case "${1}" in
-  --about)
-    echo "Runs formatters on any files that are going to be checked in."
-    ;;
-  *)
-    # make newlines the only separator
-    IFS=$'\n'
+if [[ ${1:-} == --about ]]; then
+  echo "Formats staged C++ and Objective-C++ files without staging other changes."
+  exit 0
+fi
 
-    echo "running scripts/scripts/format.sh on all staged files"
-    for file in `git diff-index --cached --name-only --diff-filter=ACMRT HEAD | grep -iE '\.(cpp|cc|h|hpp|mm)$'` ; do
-      format_file "${file}"
-      git add "${file}"
-    done
-    ;;
-esac
-}
+cd "$(git rev-parse --show-toplevel)"
+CLANG_FORMAT=${CLANG_FORMAT:-clang-format}
+shopt -s nocasematch
+
+git diff --cached --name-only --diff-filter=ACMRT -z |
+  while IFS= read -r -d '' file; do
+    case "$file" in
+      *.cpp|*.cc|*.h|*.hpp|*.mm) ;;
+      *) continue ;;
+    esac
+    read -r mode staged _ < <(git --literal-pathspecs ls-files --stage -- "$file")
+    [[ $mode == 120000 ]] && continue
+    formatted=$(git cat-file blob "$staged" |
+      "$CLANG_FORMAT" --style=file --assume-filename="$file" |
+      git hash-object -w --stdin)
+    [[ $formatted == "$staged" ]] && continue
+    # unstaged edits stay as they are; a file without any follows the index
+    if git --literal-pathspecs diff --quiet -- "$file"; then
+      follow=true
+    else
+      follow=false
+    fi
+    git update-index --cacheinfo "$mode,$formatted,$file"
+    if $follow; then
+      git checkout-index --force -- "$file"
+    fi
+  done

scripts/tests/test_format.py

@@ -0,0 +1,103 @@
+import os
+import shutil
+import subprocess
+import tempfile
+import unittest
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[2]
+FORMATTER = shutil.which(os.environ.get("CLANG_FORMAT", "clang-format"))
+
+
+@unittest.skipUnless(FORMATTER, "clang-format is required")
+class FormatHookTest(unittest.TestCase):
+    def setUp(self):
+        self.temp = tempfile.TemporaryDirectory(prefix="odr format ")
+        self.addCleanup(self.temp.cleanup)
+        self.repo = Path(self.temp.name)
+        self.run_command("git", "init", "-q")
+        (self.repo / ".clang-format").write_text("BasedOnStyle: LLVM\n")
+
+    def run_command(self, *args, **kwargs):
+        return subprocess.run(args, cwd=self.repo, capture_output=True,
+                              check=True, **kwargs)
+
+    def commit_initial(self):
+        self.run_command("git", "add", ".clang-format")
+        self.run_command("git", "-c", "user.name=Review", "-c",
+                         "user.email=review@example.invalid", "-c",
+                         "core.hooksPath=/dev/null", "commit", "-qm", "initial")
+
+    def staged(self, name):
+        return self.run_command("git", "show", f":{name}").stdout.decode()
+
+    def test_formats_only_the_staged_content(self):
+        self.commit_initial()
+        for name in ("file.cpp", "-space and\nnewline.cpp"):
+            for staged, unstaged, formatted in (
+                ("int x=1;\n", "int x=2;\n", "int x = 1;\n"),
+                ("int x = 1;\n", "int x=2;\n", "int x = 1;\n"),
+            ):
+                with self.subTest(name=name, staged=staged):
+                    path = self.repo / name
+                    path.write_text(staged)
+                    self.run_command("git", "add", "--", name)
+                    path.write_text(unstaged)
+                    result = self.run_hook()
+                    self.assertEqual(result.returncode, 0, result.stderr)
+                    self.assertEqual(self.staged(name), formatted)
+                    self.assertEqual(path.read_text(), unstaged)
+                    self.run_command("git", "rm", "--cached", "-f", "--", name)
+
+    def test_formats_a_file_without_unstaged_changes_in_place(self):
+        self.commit_initial()
+        path = self.repo / "file.cpp"
+        path.write_text("int x=1;\n")
+        self.run_command("git", "add", "file.cpp")
+        self.assertEqual(self.run_hook().returncode, 0)
+        self.assertEqual(self.staged("file.cpp"), "int x = 1;\n")
+        self.assertEqual(path.read_text(), "int x = 1;\n")
+
+    def test_formats_the_initial_commit(self):
+        (self.repo / "file.cpp").write_text("int x=1;\n")
+        self.run_command("git", "add", "file.cpp")
+        self.assertEqual(self.run_hook().returncode, 0)
+        self.assertEqual(self.staged("file.cpp"), "int x = 1;\n")
+
+    def test_a_formatter_failure_stops_the_commit(self):
+        (self.repo / "file.cpp").write_text("int x=1;\n")
+        self.run_command("git", "add", "file.cpp")
+        index = (self.repo / ".git/index").read_bytes()
+        formatter = self.repo / "failing formatter"
+        formatter.write_text("#!/bin/sh\nexit 7\n")
+        formatter.chmod(0o755)
+        self.assertNotEqual(self.run_hook(str(formatter)).returncode, 0)
+        self.assertEqual((self.repo / ".git/index").read_bytes(), index)
+
+    def run_hook(self, formatter=FORMATTER):
+        return subprocess.run(
+            [ROOT / "scripts/git_hooks/pre-commit/0_format"],
+            cwd=self.repo, capture_output=True,
+            env={**os.environ, "CLANG_FORMAT": formatter},
+        )
+
+
+class FormatScriptTest(unittest.TestCase):
+    def test_formatter_failures_stop_the_script(self):
+        with tempfile.TemporaryDirectory(prefix="odr format ") as directory:
+            root = Path(directory)
+            (root / "scripts").mkdir()
+            shutil.copy2(ROOT / "scripts/format", root / "scripts/format")
+            for name in ("src", "cli", "test/src", "python/src", "jni/src",
+                         "apple/src", "apple/include", "wasm/src"):
+                (root / name).mkdir(parents=True)
+            (root / "wasm/src/file with spaces.cpp").write_text("int x;\n")
+            formatter = root / "fake formatter"
+            formatter.write_text('#!/bin/sh\n[ "$1" = --version ] && exit 0\nexit 7\n')
+            formatter.chmod(0o755)
+            result = subprocess.run(
+                [root / "scripts/format"], cwd=directory, capture_output=True,
+                env={**os.environ, "CLANG_FORMAT": str(formatter)},
+            )
+            self.assertEqual(result.returncode, 7, result.stderr)
+            self.assertIn(b"wasm/src/file with spaces.cpp", result.stdout)