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

Parse Function Arguments as well as Command Arguments #53

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

EdwardOst
Copy link

The basic getoptions is designed for use with shell scripts but is not fully compatible for use with functions. The generated parser initializes the parameters in the outer shell rather than inside the parser function. This is ok for shell scripts, but unnecessarily pollutes the shell variables. It is a leaky abstraction.

The intent is for the parent function for which we are generating a parser to declare and bound the scope of the function parameters. Since they are only initialized rather than declared in the generated parser this will prevent pollution of the shell space.

This does not impact command scripts since the initialized variables will have shell global scope anyway, and will therefore be available to the calling script.

The change is actually a one liner moving the start of the generated parser function before the initialization of the variables.

The example shows how to use the modified getoptions to parse function arguments. It generates the parser a single time in a lazy load manner the first time the function is called.

@ko1nksm ko1nksm marked this pull request as draft July 17, 2024 12:13
@kkerce
Copy link

kkerce commented Jul 18, 2024

As I mentioned in a discussion, 99f9b41 breaks getoptions' "exit with no error status on -h, --help" functionality.

It seems this PR will need to account for how to let the caller (function) know whether -h, --help was present, so the caller can decide to return prematurely. Perhaps @EdwardOst already had an idea around that?

@ko1nksm
Copy link
Owner

ko1nksm commented Jul 19, 2024

Hi, @EdwardOst. Thanks for the contribution.

I hadn't considered using getoptions for shell function argument parsing. Because I think the command line argument syntax is redundant for shell functions. I am considering a more lightweight shell function argument syntax.

https://github.com/ko1nksm-shlab/sh-argparse

This thought has not changed, but I have found it useful to use getoptions to facilitate the creation of mocks in ShellSpec. This project was originally created as an optional parser for ShellSpec. Parsing shell function arguments could be one of the future features of getoptions, but it would probably be a lot of work.

If we limit ourselves to talking about the content of this pull request, it cannot be merged because, as @kkerce says, it would break the existing programs. Also, An example need to be simpler. Since getoptions aims to support all POSIX shells, bash-only support is not sufficient.

@ko1nksm
Copy link
Owner

ko1nksm commented Jul 20, 2024

IMO, there seems no reason to replace exit with return.

@ko1nksm
Copy link
Owner

ko1nksm commented Jul 20, 2024

Avoiding parser regenerations is a good idea, but ${FUNCNAME[0]} is a bash extension. Perhaps it would be better to generate an argument parsing function as a wrapper.

myfunc_parser_def() {
      setup   args plus:true help:usage abbr:true -- "Usage: myfunc [options...] [arguments...]" ''
      ︙
}
eval "$(getoptions-for-function ...)" # generates myfunc function

_myfunc() {
      ︙
}

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.

3 participants