Add bash 3.3+ version check to pack plugin #2358
Conversation
The pack plugin uses associative arrays (declare -A) which require bash 4.0+, but has existing checks for bash > 3 on lines 162 and 230. Add early version check to disable and return if bash < 3.3. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Both plugins already had bash version checks with early returns, but didn't explicitly disable themselves. Now they call _disable-plugin before returning to ensure consistent plugin state. - history-eternal: requires bash 4.3+ for unlimited history - percol: requires bash 4.0+ for bind -x functionality 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Changed both plugins to return 0 instead of 1/nothing and added consistent log messages explaining why the plugin is being disabled: - history-eternal: return 0 with "Disabling history-eternal plugin" message - percol: return 0 with "Disabling percol plugin" message This provides clearer feedback to users and consistent exit codes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
LGTM for the changes in history-eternal and percol. See #2354 (comment) for the changes in pack.
|
I think we should also add a global check for the requirement "Bash >= 3.2" in |
|
won't that be like making sure you are using a kernel above version 2.3 a this point? 😄 nm, added. |
There was a problem hiding this comment.
Thanks for adding checks in bash_it.sh and install.sh.
won't that be like making sure you are using a kernel above version 2.3 a this point?
Well-maintained environments shouldn't have older versions of Bash, but there are some old systems not well-maintained in academia, where the budget may not be sufficient, and where security isn't cared about too much.
It's also good to avoid loading Bash-it in other shells. It happens e.g. when a naive user puts /etc/profile.d/bash_it_loader.sh for Bash (where it should be noted that /etc/profile.d/*.sh are actually loaded by all Bourne-like shells).
Co-authored-by: Koichi Murase <[email protected]>
| if [[ -n "${BASH_VERSINFO[0]}" ]] && [[ "${BASH_VERSINFO[0]}" -eq 3 ]] && [[ "${BASH_VERSINFO[1]}" -lt 3 ]]; then | ||
| _disable-plugin pack | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
See #2354 (comment) for the changes in pack.
This hasn't been settled.
I now think this version check in pack.plugin.bash is unnecessary. Even if this change would be applied, Bash 3.3 doesn't exist because the next version of 3.2 is 4.0. Thus, the last check [[ "${BASH_VERSINFO[1]}" -lt 3 ]] is meaningless. Also, the comments I raised for the checks in bash_it.sh and install.sh apply here too.
There was a problem hiding this comment.
Bash 3.3 doesn't exist because the next version of 3.2 is 4.0.
Actually, for the same reason, the PR title should be changed to "bash 4.0+".
2 participants
Summary
Adds an early version check to the pack plugin to prevent it from loading on bash versions older than 3.3.
Changes
pack.plugin.bash_disable-plugin packbefore returning if bash < 3.3Test plan
Context
The pack plugin uses associative arrays (
declare -Aon lines 574-575) which require bash 4.0+, but had inline version checks on lines 162 and 230 checking for bash > 3. This early check prevents the plugin from loading at all on incompatible versions, providing a cleaner user experience.Related to bash version compatibility improvements across the codebase.
🤖 Generated with Claude Code