-
Notifications
You must be signed in to change notification settings - Fork 81
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
Allow --features #152
Allow --features #152
Conversation
The |
Yes, there are... but as a first iteration, I think it's fine as is. The implementation can be refined as people complain... if people complain. |
It seems ok to push them to the back anyways. Should only matter when using a combination of other features flags, which are not implemented yet. |
I did file an issue on |
@@ -302,6 +306,16 @@ program instead of cargo: | |||
$ cargo watch -x 'test -- --color=always' | |||
``` | |||
|
|||
### I want to compile my build with additional features | |||
|
|||
The `--features` flag is recognized and passed to all cargo invocations. |
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.
Ah, sorry: this is not true anymore :) given the limitations added with your latest changes.
src/lib.rs
Outdated
@@ -30,11 +30,29 @@ pub fn change_dir() { | |||
pub fn set_commands(debug: bool, builder: &mut ArgsBuilder, matches: &ArgMatches) { | |||
let mut commands: Vec<String> = Vec::new(); | |||
|
|||
// --features are injected at start of every cargo invocation |
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.
And this isn't either, given we now push at the end of the commands
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.
...which... we can't do, as per #151 (comment)
Consider cargo watch --features foo -x 'run -- --options=for --binary'
: the code as it is now would run cargo run --options=for --binary --features foo
which would not have the desired effect.
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.
This would require us to split the first word off the subcommand (if any) and inject --features foo
. Will return to the drawing board!
The features are now injected just after the subcommand. I am not fully confident in the splitting of the words, it will mess up tabs and newlines. Is this a problem? |
Hmm , I wonder if instead you could do a partial parse and split on the first whitespace, then insert in between the pieces |
Looks great! I'll test it out myself tomorrow (it's midnight here!) |
That turned out to be quite easy! Might be subtly wrong when separating with a tab instead of a space, but we are inserting For reference: |
Captures
--features <features>
and passes this to all cargo invocations.I intentionally left out
--no-default-features
and--all-features
to prevent feature creep. I think the current choice is the most common one, and for more advanced usages the workaround mentioned in #151 could be utilised.Fixes #151