Fix: Issue 71, unbound variable errors in MacOS#77
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR fixes unbound variable errors on MacOS by adding default array expansions in cli.sh and corrects an existing test by updating the expected profile count. Class diagram for updated shell script functions in cli.shclassDiagram
class parse_cli_args {
+parse_cli_args()
- Uses default array expansion for all_args, host_flags, control_flags, pass_through
- Exports CLI_HOST_FLAGS, CLI_CONTROL_FLAGS, CLI_SCRIPT_COMMAND, CLI_PASS_THROUGH
}
class process_host_flags {
+process_host_flags()
- Uses default array expansion for CLI_HOST_FLAGS
}
class debug_parsed_args {
+debug_parsed_args()
}
parse_cli_args --> process_host_flags : Calls
parse_cli_args --> debug_parsed_args : Calls
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Extract the repeated '[@]:-' fallback pattern into a small helper or variable to reduce duplication and make maintenance easier.
- Instead of hardcoding the expected profile count in the test, derive it dynamically from the PROFILE_FUNCS output to avoid breaking this test whenever profiles are added or removed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the repeated '[@]:-' fallback pattern into a small helper or variable to reduce duplication and make maintenance easier.
- Instead of hardcoding the expected profile count in the test, derive it dynamically from the PROFILE_FUNCS output to avoid breaking this test whenever profiles are added or removed.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
I just wanted to note that I pulled down amegli:71-macos-unbound-variable-error and built it this morning on an M1 Mac, and it appears to have fixed the issue for me. I was getting: |
|
No operation ID found for this PR |
|
I think you missed one in main.sh though, on line 140: If command doesn't need Docker, skip all Docker setupWhen testing locally I got this: |
|
No operation ID found for this PR |
|
You might consider array expansion syntax: |
|
No operation ID found for this PR |
|
@jeff-r-skillrev - there appear to be a number of spots that fail post-build, in addition to the main.sh line you pointed out. I'll attempt to work through them. |
|
No operation ID found for this PR |
|
@jeff-r-skillrev - I've made a number of updates and added some build/install tests. On my system this fixes a number of issues but leaves me with an "awk" error when running "claudebox". This is likely another bash incompatibility issue. Let me know if you see the same. |
|
No operation ID found for this PR |
|
@amegli I rebuilt everything ( I'll keep using it today and tomorrow, to see if I find other bugs. But from my perspective, this bug fix is working. |
|
No operation ID found for this PR |
|
I think I might have reproduced your awk issues. Like you said, running just claudebox does it. |
|
No operation ID found for this PR |
|
I will say, I launched claudebox, gave it the awk error, and it found a decent workaround. Here is the patch it produced, which is now letting me use claudebox on my Macbook M1. |
|
No operation ID found for this PR |
Installation on MacOS produces a number of "unbound variable" errors from "cli.sh" - #71. These changes apply default expansions for a few arrays in that file. An existing failing test was also corrected by adjusting the expected profile name count.
Update
This PR has grown as fixes uncovered additional issues. There are now a number of updated bash files, almost all centered around "unbound variable" errors, and a new "test_install.sh" file to try and ensure the build/install process works in multiple bash versions (it did recreate some of the "unbound variable" errors). So far build, install, and basic usage in MacOS seems to work.
Summary by Sourcery
Prevent unbound variable errors on MacOS by adding default expansions for arrays in cli.sh and update the profile name count in the compatibility test.
Bug Fixes:
Tests: