Skip to content

#1875 fix: declare bash in pm.sh and check the sh dialect - #1887

Open
RAprogramm wants to merge 1 commit into
HyDE-Project:devfrom
RAprogramm:1875
Open

#1875 fix: declare bash in pm.sh and check the sh dialect#1887
RAprogramm wants to merge 1 commit into
HyDE-Project:devfrom
RAprogramm:1875

Conversation

@RAprogramm

@RAprogramm RAprogramm commented Jul 30, 2026

Copy link
Copy Markdown

Closes #1875.

Configs/.local/lib/hyde/pm.sh declared #!/usr/bin/env sh while using an indexed array (ARGS+=("$1"), "${ARGS[@]}") and local in 27 functions. It now declares bash, which is what the file needs and what hyde-shell already runs it with. Running it directly on a system where /bin/sh is dash stops being a syntax error.

tests/test_shell.sh gains the pass that could not be enabled while that file stood as it was: for every script whose shebang names sh, shellcheck must report no SC3xxx finding. That family is the dialect one — a construct the declared shell does not have — and it sits below error severity, so the existing severity-filtered run never saw it. The comment that explained why the suite had to ignore the mismatch is replaced by one explaining what it now enforces.

Verified with sh tests/run.sh: 8 cases, 0 failed, 106 files parsed. The dialect pass reports as skipped here because shellcheck is not installed on this machine, so its first real run is CI.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected shell script execution to consistently use Bash, preventing compatibility issues with Bash-specific syntax.
  • Tests

    • Improved shell compatibility validation.
    • Added checks to detect shell scripts labeled as sh that use unsupported constructs.
    • Added reporting for the number of scripts validated against the sh dialect.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3a32eda7-a045-45e7-aa35-9c2283d5f26e

📥 Commits

Reviewing files that changed from the base of the PR and between bab9fcf and 4d8f10e.

📒 Files selected for processing (2)
  • Configs/.local/lib/hyde/pm.sh
  • tests/test_shell.sh

📝 Walkthrough

Walkthrough

The change updates pm.sh to declare Bash execution and extends shell tests with shellcheck-based validation for scripts declaring the sh dialect.

Changes

Shell dialect alignment

Layer / File(s) Summary
Declare Bash execution
Configs/.local/lib/hyde/pm.sh
The script shebang changes from sh to bash.
Validate sh declarations
tests/test_shell.sh
Comments clarify Bash parsing, and a new pass checks sh-declared scripts for SC3xxx diagnostics and reports the checked-script count.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Poem

A bashy bunny hops with glee,
“No dash-side tricks to trouble me!”
Shellcheck counts the scripts in line,
While honest shebangs neatly shine.
Hop, hop—clean shells by design!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: switching pm.sh to Bash and adding shell dialect validation.
Linked Issues check ✅ Passed The changes satisfy #1875 by fixing pm.sh's shebang and adding a ShellCheck pass for sh-dialect scripts.
Out of Scope Changes check ✅ Passed All modified files directly support the shebang fix or the new dialect check, with no unrelated changes evident.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant