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

Always accept = or space as delimiters when parsing common command line parameters. #13857

Merged
merged 9 commits into from
Oct 7, 2020

Conversation

BillyONeal
Copy link
Member

Resolves #11456.

Also rename $args to $testArgs because the former is a PowerShell builtin variable.
@BillyONeal BillyONeal added category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:internal This PR or Issue was filed by the vcpkg team. labels Oct 2, 2020
@BillyONeal
Copy link
Member Author

CI failed because repo.msys.org is down.

Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

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

Those are a few of the things I've noticed, but the big thing I'd like to see is this working for options to commands. This isn't necessary for this first pass, however. The enum class thing is a necessity before I approve.

toolsrc/src/vcpkg/vcpkgcmdarguments.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/vcpkgcmdarguments.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/vcpkgcmdarguments.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

I'd like to see an enum class over size_t as the return of try_parse_argument_as_option(); otherwise LGTM.

@BillyONeal
Copy link
Member Author

@strega-nil Do you have an example command like you want to work for your more complete desired overhaul that doesn't work with the current state of this PR? Out of band you said --x-feature but that option doesn't seem to exist...

@strega-nil
Copy link
Contributor

@BillyONeal --x-feature is an option for the install command, not a global option: https://github.com/microsoft/vcpkg/blob/master/toolsrc/src/vcpkg/install.cpp#L549

@BillyONeal
Copy link
Member Author

@BillyONeal --x-feature is an option for the install command, not a global option: https://github.com/microsoft/vcpkg/blob/master/toolsrc/src/vcpkg/install.cpp#L549

I tried it with the install command and that didn't work. But I didn't try with a manifest so that likely explains it ¯_(ツ)_/¯

@strega-nil
Copy link
Contributor

Ah, yeah, it's a manifest-only option.

@BillyONeal BillyONeal changed the title Always accept = or space as delimiters when parsing command line parameters. Always accept = or space as delimiters when parsing common command line parameters. Oct 7, 2020
@BillyONeal BillyONeal merged commit ab86954 into microsoft:master Oct 7, 2020
@BillyONeal BillyONeal deleted the command_line_root branch October 7, 2020 17:31
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vcpkg is inconsistent in how it accepts path parameters in its command line interface
3 participants