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

refactor: args to flags #22

Merged
merged 3 commits into from
Dec 20, 2021
Merged

refactor: args to flags #22

merged 3 commits into from
Dec 20, 2021

Conversation

bcoe
Copy link
Collaborator

@bcoe bcoe commented Dec 4, 2021

Based on #12, and sifting through online conversations like this, this refactor
proposes using the terminology "args" for options that take arguments,
and "flags" for options that are boolean.

Fixes #12

Based on #12, and sifting through online conversatino [like this](https://unix.stackexchange.com/questions/285575/whats-the-difference-between-a-flag-an-option-and-an-argument), this refactor
proposes using the terminology "args" for options that take arguments,
and "flags" for options that are boolean.

Fixes #12
@bcoe
Copy link
Collaborator Author

bcoe commented Dec 4, 2021

CC: @shadowspawn

@shadowspawn
Copy link
Collaborator

Short version: I think it is an improvement. 👍

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

I would say that "args" is an umbrella term that includes flags. it's "yargs", not "yvalues" or "yparams" :-p

@shadowspawn
Copy link
Collaborator

To expand a little:

  • I think flags is much better than using args for the boolean options
  • this PR uses "args" for the "option arguments" which is a justification but a little subtle. Could be optionArgs, but then should it be optionFlags for symmetry? Names are hard! (And I care.)
  • I was ok with values

I would say that "args" is an umbrella term that includes flags.

Seconding that viewpoint, the routine we are implementing is called parseArgs. 🙂

@shadowspawn
Copy link
Collaborator

Of note for consistency, the configuration property for setting up the parsing is called withValue.

@bcoe
Copy link
Collaborator Author

bcoe commented Dec 5, 2021

so we're leaning towards flags and values? and dropping uses of args?

@bcoe bcoe removed the do-not-merge label Dec 17, 2021
@bcoe
Copy link
Collaborator Author

bcoe commented Dec 17, 2021

@iansu @shadowspawn @ljharb take a look.

@shadowspawn shadowspawn changed the title refactor: args to flags, values to args refactor: args to flags Dec 17, 2021
@shadowspawn
Copy link
Collaborator

LGTM

(Changed title to match latest code.)

Copy link
Collaborator

@shadowspawn shadowspawn left a comment

Choose a reason for hiding this comment

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

👍

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.

Finalize variable names for arguments returned by parse
5 participants