-
Notifications
You must be signed in to change notification settings - Fork 460
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
Add support for parsing *FLAGS
with shlex
#1181
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
just some minor styling suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks LGTM!
Just want @thomcc to double check this, since it involves env var, which is kind of like a new interface, and new API.
Given that we never plan on release v2 I'd like to be a little bit more careful on that.
I'm wary of adding a dependency since it literally always causes us big problems but I don't see why this one would so I suppose we can give it a shot. I think I'd prefer this be optional. We probably also shouldn't mention |
Good idea, I changed the env var to be
Do you mean the dependency being optional / gated behind a feature? If it might hurt the user experience a bit, but I think it would be fine since callers of |
Given how small shlex is (no dependency, no build-script), I don't think we need to add a feature for it. In the future we could vendor it, given that the code for splitting based on shell isn't very large, so I don't think it's necessary. |
Given that there's no objection, I'd merge it in |
Hello,
This PR adds support for parsing
*FLAGS
with theshlex
crate. This effectively fixes #847, and means*FLAGS
with spaces designed formeson
,make
andcmake
will work with no extra configuration.In order to preserve compatibility with existing setups, the behavior must be opted into with
CC_PARSE_FLAGS_WITH_SHLEX
, or theparse_flags_with_shlex
method on theBuilder
.