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

Support options with only a short flag #1256

Merged
merged 5 commits into from May 26, 2020

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Apr 25, 2020

Pull Request

Problem

Users may wish to declare an option which has only a short flag and no long flag.

Defining just a short flag for an option appeared to almost work, but was not actually supported internally.

See: #1249

Solution

Extend option flag parsing to allow short, or long, or both. Extend the support to the custom option flags for .helpOption() and .version() too.

ChangeLog

@shadowspawn shadowspawn added enhancement semver: major Releasing requires a major version bump, not backwards compatible labels Apr 25, 2020
@shadowspawn shadowspawn changed the base branch from develop to release/6.x May 25, 2020 08:09
@shadowspawn shadowspawn changed the title WIP: Support options with only a short flag Support options with only a short flag May 25, 2020
@shadowspawn shadowspawn marked this pull request as ready for review May 25, 2020 08:11
@shadowspawn
Copy link
Collaborator Author

Side note: I hit a merge conflict when updating the branch which prompted me to look at how negate was set in the option constructor. I tightened the test to match the README and expectation in other code that it is for a long option that startsWith --no-.

@shadowspawn
Copy link
Collaborator Author

I created a release/6.x branch since this is a semver major change, although hopefully only breaking for people who were using lone short flags and working around the bugs.

I am thinking we might do a pre-release in June for v6, with short flags and variadic options.

Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for doing this.

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label May 26, 2020
@shadowspawn shadowspawn merged commit a31bb3f into tj:release/6.x May 26, 2020
@shadowspawn shadowspawn deleted the feature/short-flag branch June 1, 2020 09:43
@shadowspawn shadowspawn added this to the v6.0.0 milestone Jun 20, 2020
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Jul 19, 2020
@shadowspawn shadowspawn mentioned this pull request Dec 27, 2021
let longFlag;
// Use original very loose parsing to maintain backwards compatibility for now,
// which allowed for example unintended `-sw, --short-word` [sic].
const flagParts = flags.split(/[ |,]+/);
Copy link

Choose a reason for hiding this comment

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

this pattern seems buggy if the comment is still accurate, did you intend to say /( |,)+/ there? or just /[ ,]+/ to match space or comma, or really wanted to add a pipe | to the mix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The pipe is really in the mix. From the README:

Each option can have a short flag (single character) and a long name, separated by a comma or space or vertical bar ('|').

The comment is about -sw which is neither a short flag nor a long name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement semver: major Releasing requires a major version bump, not backwards compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants