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

Allow flags to precede required positional arguments #124

Closed
DannyBen opened this issue Oct 15, 2021 · 8 comments · Fixed by #139
Closed

Allow flags to precede required positional arguments #124

DannyBen opened this issue Oct 15, 2021 · 8 comments · Fixed by #139
Labels
enhancement New feature or request

Comments

@DannyBen
Copy link
Owner

DannyBen commented Oct 15, 2021

Some users expect to be able to run cli --arg param required_arg in addition to the currently supported cli required_arg --arg param.

Any reasonable solution will probably require changing the serial filters here:

<%= render(:required_args_filter).indent 2 %>
<%= render(:required_flags_filter).indent 2 %>
<%= render(:parse_requirements_while).indent 2 %>

so that they just work together as one, probably something like this:

  1. Go through each argument in the command line
  2. If it is a --flag, add it to the ${args[--flag]} array
  3. If it is an arg, add it to the ${args[arg]} array, where the arg name is shifted from the full args list.
  4. After all that is done, add a single filter that just checks that both required args and flags are present, and abort if not.
@cjbrigato
Copy link

cjbrigato commented Oct 17, 2021

Note: that might feet in a more general issue/discussion, feel free to tell me where to move it or to do it yourself.


The piece that makes most of us presuming pre-operand options :
POSIX(2018) Basedef - 12. Utility Conventions
as some interesting reminders that may benefit much to your work :

Guideline 9:
All options should precede operands on the command line.

While not enforcing pre-operand option feels like a good relaxe from the standards,
not supporting at all what is expected to be enforced is quite a break.
You wrote you have at heart the user feeling, and I'm quite confident this is an important drawback that would limit really badly adoption of such a great tool.

Also, regarding option at the end, they are historically enforced before operands to deal easily with the ability to distinguish dashes from option versus operands.
A definitive addition to these is

Guideline 10:
The first -- argument that is not an option-argument should be accepted as a delimiter indicating the end of options. Any following arguments should be treated as operands, even if they begin with the '-' character.

This is highly convential thing that solves so many problems and might be implemented in the same PR.

Additionnally but less important, there are some other guidelines that are very practical when supported by cli parsing :

Guideline 8:
When multiple option-arguments are specified to follow a single option, they should be presented as a single argument, using characters within that argument or characters within that argument to separate them.

Being able to tell a list without having to quote it is realy satisfying (but I recon this is nowadays rarely implemented unless specifically said, or partially implemented (e.g only when using the awfull = and comma separated list).

Guideline 12:
The order of operands may matter and position-related interpretations should be determined on a utility-specific basis.

Enforcing every "non option argument" as positional is very limiting. All in all, positional arguments are limitating, and quite rare unless you manipulate e.g. files, and again most of the time the only position considered is "all the previous operands" vs the last one as to distinguish a list from it's containers (e.g cp/mv/ln etc).
Positional arguments are rarely felt as cool

Also pure positionals or option at end breaks the ability to do things like command line after the -- in docker run

Guideline 13:
For utilities that use operands to represent files to be opened for either reading or writing, the '-' operand should be used to mean only standard input (or standard output when it is clear from context that an output file is being specified) or a file named -

Unless I again fail to find immediate implementation, I feel that actual bashly state breaks the ability to take stdin as input this way.

@DannyBen
Copy link
Owner Author

Well - bashly never claimed to be compliant with any standard, nor does it strive to do so.

Lets not forget, that this is a bash script after all, and it is unlikely to support everything that can be done in the command line.

We have enabled the Discussions tab, so we have a better place to discuss such things without the obligation to "solve" them :)

@cjbrigato
Copy link

Not at all telling that posix should be followed. Bash is not posix, and that's what we talk about.
It's just that this chapter is felt like a foundation of what we are used to and expect. That could also be a very good list of things not to do at all :)
"No option argument should be optional" is perfect example of some posix nonsense in the sole name of a group's opinion on how to make things portable. And probably my next custom t-shirt.

@DannyBen DannyBen changed the title Consider allowing flags to precede the positional arguments Allow flags to precede required positional arguments Oct 28, 2021
@DannyBen
Copy link
Owner Author

I just realized that the problem is only with positional arguments that are marked as required.

@DannyBen
Copy link
Owner Author

@cjbrigato - I have implemented a change that fixes this, it is merged to the master branch.
If you wish to test it prior to release, feel free to use this version, if not, it will be added to the next release, in a few days.

Thanks for this issue, it was an important change.

@cjbrigato
Copy link

Thanks a lot, it's both working great and highly appreciated.

@DannyBen
Copy link
Owner Author

DannyBen commented Nov 4, 2021

Excellent, I am happy to hear and thanks for reporting.

@DannyBen
Copy link
Owner Author

@cjbrigato - I have added support for stdin (by simply allowing - as argument) in #160.

It will be available in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants