fix(regen-defs): Fix globs and clean tempfiles on INT, TERM - #381

Pull request overview

This PR adds signal handling to the regen-defs.zsh script to clean up temporary files when interrupted, and uses eval to fix glob expansion in cpplint commands. The script processes .def test fixture files that contain cpplint commands with glob patterns (like src/*) and their expected outputs.

• Added trap handler for INT and TERM signals to clean up temporary files on interruption
• Changed command execution to use eval to properly expand glob patterns in the commands
• Refactored cleanup logic into a dedicated function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Actionable comments posted: 1

Actionable comments posted: 0

The signal handler cleans the temp files but does not terminate the script. In zsh, a handled INT or TERM resumes after the trap; a minimal reproduction prints CLEANED and then CONTINUED_AFTER_INT.

Here that means regen-defs.zsh can continue after Ctrl-C and rewrite the current .def file from partial output. Please have the handler clean up and exit with the matching signal status, and add a small regression check for the interrupted path.

Nice to see the trap actually terminating now, but I think 53a1696 has a side effect: exit went into cleanup, and cleanup is also what replaced the rm at the end of each loop iteration.

# Clean up temporary files
cleanup      # <- ends in `exit`, so the success path exits too

So the run stops after the first .def file instead of continuing through the rest.

Also, trap cleanup INT TERM invokes the function with no arguments — $1 is only set in the TRAPINT()-style form — so exit $((128 + $1)) isn't valid arithmetic on either path.

Splitting the two responsibilities keeps the loop intact and still satisfies the signal-status request:

cleanup() { rm -f "$stdout_file" "$stderr_file"; }
trap 'cleanup; exit 130' INT
trap 'cleanup; exit 143' TERM

With the end-of-iteration call left as plain cleanup. I verified this control flow in bash (loop completes; INT exits 130) — I don't have the environment here, so you'll want to confirm on your side.

Unrelated to the title: e89746c swaps $HOME/Documents/cpplint for $HOME/Projects/cpplint, which just trades one local layout for another. Deriving it would work for everyone:

cpplint="${0:A:h}/cpplint.py"

(set before the cd samples/).

eval also drops the argument boundary provided by the quotes around $cpplint. For example, with:

cpplint='/tmp/cpplint checkout/cpplint.py'
cmd='--flag'
eval uv run "$cpplint" $cmd

the second parse passes /tmp/cpplint and checkout/cpplint.py as separate arguments. The previous non-eval form passed the configured path as one argument. This affects any checkout path containing whitespace, and deriving the path from the script location would still have the same problem if a parent directory contains a space.

Could the command expansion preserve $cpplint as one shell word through the second parse (for example, using zsh-safe quoting), with a regression case for a path containing whitespace? The Python suite is green on both the exact head (222 passed) and the current merge ref (231 passed), but it does not exercise regen-defs.zsh.