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

added required options #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

added required options #33

wants to merge 1 commit into from

Conversation

zinccyy
Copy link

@zinccyy zinccyy commented Aug 25, 2020

Hi, I added a required options feature by adding a new field parsed to the option struct and a OPT_REQUIRED flag so that every parsed option is checked after parsing so if the option is required and if it doesn't exist the program exits. Unfortunately, by the need to change the field in a struct I had to remove the const attribute of the struct parameters.

@cofyc-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mcindrich
To complete the pull request process, please assign cofyc
You can assign the PR to them by writing /assign @cofyc in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cofyc-bot cofyc-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 25, 2020
@zinccyy
Copy link
Author

zinccyy commented Aug 25, 2020

/assign @cofyc

int flags)
{
const char *s = NULL;
opt->parsed = 1;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why setting this for all arguments?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I thought it would be the simplest solution to the problem. You set it to 1 for arguments that are used at the command line and at the end of parsing you simply check if the required options have been parsed (meaning used in this context).

@@ -80,14 +84,15 @@ struct argparse_option {
argparse_callback *callback;
intptr_t data;
int flags;
int parsed;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need this, you define it as a flag

Copy link
Owner

@cofyc cofyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how to use this feature, please update test_argparse.c too.

@zinccyy
Copy link
Author

zinccyy commented Aug 26, 2020

I didn't update test_argparse.c because it would then break the current tests. You use this option by providing OPT_REQUIRED flag to the option flags. For example: OPT_BOOLEAN('f', "force", &force, "force to do", NULL, 0, OPT_REQUIRED),.

@zinccyy zinccyy closed this Aug 26, 2020
@zinccyy zinccyy reopened this Aug 26, 2020
@feliwir
Copy link

feliwir commented Apr 14, 2022

Hi, I added a required options feature by adding a new field parsed to the option struct and a OPT_REQUIRED flag so that every parsed option is checked after parsing so if the option is required and if it doesn't exist the program exits. Unfortunately, by the need to change the field in a struct I had to remove the const attribute of the struct parameters.

i like the design of just having to add a flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants