docs: explain unmatched Bash glob behavior - #12
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new Bash example demonstrating the use of nullglob to handle empty glob matches, along with corresponding documentation updates in the README and reference guide. The feedback highlights a compatibility issue with Bash 3.2 (default on macOS) where EXIT traps defined inside subshell functions do not trigger, resulting in temporary directory leaks. It is recommended to move the temporary directory creation and trap definitions to the global scope.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Fixed in e2e48f6. The temporary directory and EXIT trap now live at script scope, so cleanup is not tied to the main subshell on Bash 3.2. Verified locally with bash -n, an isolated TMPDIR cleanup check, and git diff --check. I could not run task ci locally because the required Task/ShellCheck/shfmt/bats toolchain is unavailable in this environment. |
|
Thank you for your contribution. I will review it carefully and come back with my feedback soon. |
posidoni
left a comment
There was a problem hiding this comment.
Thanks for this — closes #9 exactly per the contract in CONTRIBUTING.md, and the reference/bash.md addition on nullglob/failglob is accurate and well cited. CI is green on all four required checks.
Nit: examples/bash/04-nullglob-empty-match.bad.sh still defines the mktemp/trap EXIT pair inside the main() ( ... ) subshell (as gemini-code-assist flagged). Under Bash 3.2 — which macOS ships by default and which this repo's shell-macos CI job targets — an EXIT trap set inside a subshell doesn't fire when the subshell exits, so the temp dir leaks. .good.sh already has the fix (trap hoisted to global scope); .bad.sh just needs the same move. This is a portability nit, not a blocker — approving as-is and I'll track it in a small follow-up issue/PR rather than hold this up.
Merging now. Thanks for picking this up!
|
Merged — thank you for the contribution, @cnYui! Nice work on the nullglob write-up and example pair. Appreciate you taking the time. |
Summary
nullglobfixnullglobandfailglobin the Bash referenceCloses #9.
Validation
task citask hooksgit diff --checkfound: *.txt; good printsmatched 0 file(s)