Skip to content

fix(pre-commit): preserve matched filenames in no-underscore-md hook#458

Open
ooooo-create wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
ooooo-create:fix/pre-commit-no-underscore-md-hook
Open

fix(pre-commit): preserve matched filenames in no-underscore-md hook#458
ooooo-create wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
ooooo-create:fix/pre-commit-no-underscore-md-hook

Conversation

@ooooo-create
Copy link
Copy Markdown

Without --, the first matched filename is treated as $0 by bash -c, which means the hook can fail without reporting every offending Markdown file.

  • pass -- to bash -c in the no-underscore-md local pre-commit hook
  • add an inline comment explaining why the separator is required
bash -c 'echo "Hi! $@"' "foo" "bar"
# Hi! bar

bash -c 'echo "Hi! $@"' -- "foo" "bar"
# Hi! foo bar

I found this issue in NVIDIA-NeMo/Automodel@a409ff1#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9 , and verified the fix in the NVIDIA-NeMo/Automodel.

Because I noticed that several repositories under NVIDIA-NeMo use this hook, I came across this repository as well. I'd be happy to submit the same fix to other affected repositories too.

Copilot AI review requested due to automatic review settings April 20, 2026 06:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a subtle argument-passing bug in the template’s local no-underscore-md pre-commit hook so all matched Markdown filenames are correctly preserved and reported.

Changes:

  • Pass -- to bash -c so the first matched filename isn’t consumed as $0.
  • Add an inline comment documenting the bash -c argument behavior and why -- is required.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants