oh-my-bash icon indicating copy to clipboard operation
oh-my-bash copied to clipboard

Fixed _omb_cd_dirstack[0]: unbound variable. Fixes #716.

Open gurelkaynak opened this issue 3 months ago • 3 comments

User description

Fixes #716


PR Type

Bug fix


Description

  • Fixed unbound variable error in directory stack handling

  • Added parameter expansion with default value to prevent bash error


Diagram Walkthrough

flowchart LR
  A["Array Access"] -- "Add Default Value" --> B["Safe Parameter Expansion"]
  B -- "Prevents" --> C["Unbound Variable Error"]

File Walkthrough

Relevant files
Bug fix
directories.sh
Fix unbound variable in directory stack array                       

lib/directories.sh

  • Added parameter expansion with default value to _omb_cd_dirstack[0]
  • Changed ${_omb_cd_dirstack[0]} to ${_omb_cd_dirstack[0]-}
  • Prevents unbound variable error when array is empty
+1/-1     

gurelkaynak avatar Sep 30 '25 07:09 gurelkaynak

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

716 - Partially compliant

Compliant requirements:

  • Prevent 'unbound variable' error for _omb_cd_dirstack[0] in non-interactive shells.
  • Implement a safe guard consistent with 'nounset' environments.

Non-compliant requirements:

  • Ensure cd works reliably without breaking existing directory stack behavior.

Requires further human verification:

  • Verify interactively and in non-interactive shells (e.g., Cursor agent) that cd works and directory stack behavior remains unchanged.
  • Test with set -u enabled to confirm no regressions elsewhere in directories.sh.
⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Behavior Change

Confirm that using ${_omb_cd_dirstack[0]-} preserves previous logic when the array is empty and does not inadvertently skip prepending $PWD or alter DIRSTACK semantics in edge cases.

[[ ${_omb_cd_dirstack[0]-} == "$PWD" ]] ||
  _omb_cd_dirstack=("$PWD" "${_omb_cd_dirstack[@]}")
builtin cd "$@" &&
  _omb_cd_dirstack=("$PWD" "${_omb_cd_dirstack[@]}")

PR Code Suggestions ✨

No code suggestions found for the PR.

If we could get this reviewed that would be superduper so I can get back on master :)

rockiesmagicnumber avatar Oct 23 '25 00:10 rockiesmagicnumber