internal: remove invocationLocation in favor of invocationStrategy#17888
internal: remove invocationLocation in favor of invocationStrategy#17888bors merged 1 commit intorust-lang:masterfrom
invocationLocation in favor of invocationStrategy#17888Conversation
rustbot
added
the
S-waiting-on-review
|
I changed both |
Right, looks like we don't need the variants to hold that info at all |
Tyrubias
force-pushed
the
remove-invocation-location
branch
from
e169687 to
b5ae2c6
Compare
…ategy` These flags were added to help rust-analyzer integrate with repos requiring non-Cargo invocations. The consensus is that having two independent settings are no longer needed. This change removes `invocationLocation` in favor of `invocationStrategy` and changes the internal representation of `InvocationStrategy::Once` to hold the workspace root.
Tyrubias
force-pushed
the
remove-invocation-location
branch
from
b5ae2c6 to
b0f20c7
Compare
|
Thanks! |
|
☀️ Test successful - checks-actions |
lnicola
changed the title
invocationLocation in favor of invocationStrategyinvocationLocation in favor of invocationStrategy
| cargo_buildScripts_invocationLocation: InvocationLocation = InvocationLocation::Workspace, | ||
| /// Specifies the invocation strategy to use when running the build scripts command. | ||
| /// If `per_workspace` is set, the command will be executed for each workspace. | ||
| /// If `once` is set, the command will be executed once. |
There was a problem hiding this comment.
This should document the working directory that the command will be executed in, for both cases.
| @@ -3196,14 +3158,6 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json | |||
| "The command will be executed once." | |||
There was a problem hiding this comment.
Might be good to also mention the effect on the working directory here.
| )) | ||
| } | ||
| }; | ||
| let current_dir = workspace_root; |
There was a problem hiding this comment.
I am surprised by this; once should not be per-workspace... or does this refer to the vscode concept of a workspace rather than the cargo concent?
The argument list for this function is quite confusing since there are multiple "workspaces" but then just a single "workspace_root"...
There was a problem hiding this comment.
The variable name here is a bit misleading (should be renamed into current_dir/working_dir, the sole caller of this passes the actual root here.
minor: Improve documentation for `InvocationStrategy` cc #17888
minor: Improve documentation for `InvocationStrategy` cc rust-lang/rust-analyzer#17888
…tings, r=jieyouxu Remove unncessary option for default rust-analyzer setting In favor of rust-lang/rust-analyzer#17888
…tings, r=jieyouxu Remove unncessary option for default rust-analyzer setting In favor of rust-lang/rust-analyzer#17888
…tings, r=jieyouxu Remove unncessary option for default rust-analyzer setting In favor of rust-lang/rust-analyzer#17888
Rollup merge of rust-lang#132438 - chenyukang:yukang-fix-analyzer_settings, r=jieyouxu Remove unncessary option for default rust-analyzer setting In favor of rust-lang/rust-analyzer#17888
5 participants
These flags were added to help rust-analyzer integrate with repos requiring non-Cargo invocations. The consensus is that having two independent settings are no longer needed. This change removes
invocationLocationin favor ofinvocationStrategyand changes the internal representation ofInvocationStrategy::Onceto hold the workspace root.Closes #17848.