Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for nested function arguments. #31

Merged
merged 8 commits into from
Dec 17, 2024
Merged

Adds support for nested function arguments. #31

merged 8 commits into from
Dec 17, 2024

Conversation

UrsaDK
Copy link
Owner

@UrsaDK UrsaDK commented Dec 15, 2024

This PR reworks the default argument logic allowing it to collect arguments just for the current function, using BASH_ARGC as a boundary. It’s designed to make things more reliable in nested contexts, allowing the user to work only with the arguments relevant to the current function call.

billyzkid and others added 8 commits August 24, 2023 10:42
This PR updates the logic executed when arguments are not explicitly
passed to `getopts_long` and instead they are determined from the call
stack (using `BASH_ARGV`), which attempts to match the default argument
behavior of the builtin `getopts` command.

Based on my tests, the updated logic fixes all the bugs I found, e.g.
when calling `getopts_long` within nested functions, etc.

Also, while testing, I noticed the use of `BASH_ARGV` requires `shopt -s
extdebug` to be enabled. Also, the logic may not work in all scenarios,
e.g. when the arguments have been shifted, etc. However, this was always
the case, and is not new to the changes in this PR.
@UrsaDK UrsaDK merged commit 737a918 into master Dec 17, 2024
3 checks passed
@UrsaDK UrsaDK deleted the gh-17 branch December 17, 2024 14:42
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (9b94aad) to head (a892118).
Report is 1 commits behind head on master.

Additional details and impacted files

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