-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Generalize flycheck to arbitrary commands #3804
Conversation
Merge conflict |
} | ||
FlycheckConfig::CustomCommand { command, args } => { | ||
let mut cmd = Command::new(command); | ||
cmd.args(args); |
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.
I think it would be nice to support --manifest-path
here as well. Maybe some special arg like MANIFEST_PATH
that will be replaced here with the manifest path?
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.
Why the user can't add --manifest-path=/path/to/Cargo.toml
themselves?
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 some dynamic configuration, while the other way would support having a static configuration. It would be solvable, but I think it requires more work for the user than doing it here.
bors r+ |
Build succeeded |
We don't dynamically decide on Cargo.toml ourselves yet, we always use just
the first Cargo.toml there is. For minimizing the work, the user should use
the `CargoCommand` option, the purpose of `CustomCommand` is just to be
able to tweak anything, so optimizing user experience for it is not a
priority.
That said, long term we probably should support various $PROJECT
$CARGO_TOML $FILE placeholders in similar places...
…On Wed, 1 Apr 2020 at 13:19, Bastian Köcher ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In crates/ra_flycheck/src/lib.rs
<#3804 (comment)>
:
> - args.extend(self.options.args.iter().cloned());
+ let mut cmd = match &self.config {
+ FlycheckConfig::CargoCommand { command, all_targets, extra_args } => {
+ let mut cmd = Command::new(cargo_binary());
+ cmd.arg(command);
+ cmd.args(&["--workspace", "--message-format=json", "--manifest-path"]);
+ cmd.arg(self.workspace_root.join("Cargo.toml"));
+ if *all_targets {
+ cmd.arg("--all-targets");
+ }
+ cmd.args(extra_args);
+ cmd
+ }
+ FlycheckConfig::CustomCommand { command, args } => {
+ let mut cmd = Command::new(command);
+ cmd.args(args);
This would require some dynamic configuration, while the other way would
support having a static configuration. It would be solvable, but I think it
requires more work for the user than doing it here.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3804 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANB3M35EZTTZGGT5BJ6S7TRKMPN7ANCNFSM4LYYRZBA>
.
|
bors r+
🤖