Skip to content

fix: filter syntax check by shebang to skip python3/zsh files - #140

Closed
dsaikou618 wants to merge 3 commits into
masterfrom
fix/verify-shebang-filter
Closed

dsaikou618 wants to merge 3 commits into
masterfrom
fix/verify-shebang-filter

Conversation

@dsaikou618

Copy link
Copy Markdown
Collaborator

The script previously ran on all files found in the stow directories, including python3 and zsh scripts which would false-positive as syntax errors.

This fix adds a shebang filter: which skips files that don't have a bash/sh shebang, so only actual bash shell scripts are checked.

Closes the false-positive issue where python3 and zsh scripts in the stow tree would trigger syntax error reports.

@dsaikou618
dsaikou618 requested a review from TriTacLe as a code owner October 27, 2026 10:07
@dsaikou618
dsaikou618 force-pushed the fix/verify-shebang-filter branch from 3aba915 to 9b3263a Compare October 27, 2026 10:12
@TriTacLe

Copy link
Copy Markdown
Owner

The shebang filter is the right idea but the regex only matches a shebang line ending exactly in "bash" or "sh": ^#!.*[ /](bash|sh)$. A shebang with trailing flags, like #!/bin/bash -e or #!/usr/bin/env -S bash -euo pipefail, no longer ends in "bash"/"sh" and gets silently skipped from syntax checking. No script in the repo currently uses flags, so this doesn't break anything today, but it's a latent gap in exactly the check this PR exists to fix. Consider matching the interpreter name anywhere after bash/sh/-S, e.g. ^#!.*[ /](bash|sh)([ \t]|$).

This PR is also stacked on #139's unmerged branch, so the diff here includes all of that PR's open issues too - worth rebasing once #139 lands.

Copy link
Copy Markdown
Owner

Closing: the unique fix here is already on master.

The verify-system.sh shebang filter landed in #138, which is merged. Your implementation (grep -qE '^#!.*[ /](bash|sh)$') and the merged one are equivalent, so nothing is lost, but this branch now conflicts with master in that exact file.

Everything else in this PR is the shared profiles base it has in common with #139 and #141, which carries four reverts against current master. Full review on #141.


Generated by Claude Code

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