bug: Use manifest path as reported buildfile#38
Merged
Conversation
Previously, we were reporting the workspace Cargo.toml as the buildfile for a `discover` invocation. This led to incorrect behavior where RA would "unindex" any crates that were included in the first `discover` invocation but not in subsequent invocations (reporting the buildfile as the workspace Cargo.toml implies that we are telling RA about all of the crates in the workspace, but we are actually only telling it about a subset). Consider the following dependency graph: ``` A: C B: C C: ``` If we're invoked with a file from crate `A`, we would tell RA about crates `A` and `C` with the workspace manifest as the buildfile. If we are subsequently invoked with a file from crate `B`, we'd still report the buildfile as the workspace manifest, but now we'd only tell RA about crates `B` and `C`. This causes RA to "unindex" `A`, since we're telling RA that the workspace doesn't include it. This commit rectifies the issue by reporting the current crate's Cargo.toml as the buildfile in the `discover` output. However, this changes the semantics of how and when `check` is invoked. In the above example, we'd now report two separate "projects" after being invoked with files from crates `A` and `B`. Because `A` and `B` do not depend on each other, RA will invoke `check` separately for each of them. This is rather annoying if they share many dependencies (e.g. if `C` has a giant dependency tree), since those `check` invocations will be duplicating a lot of work. This behavior can be adjusted by setting RA's `check.invocationStrategy` setting to `"once"`, which tells it just to invoke check for the current project instead of all the projects we've told it about. This commit also updates the README to reflect this change. Fixes #34
8af68ec to
8a57a66
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, we were reporting the workspace Cargo.toml as the buildfile
for a
discoverinvocation. This led to incorrect behavior where RAwould "unindex" any crates that were included in the first
discoverinvocation but not in subsequent invocations (reporting the buildfile as
the workspace Cargo.toml implies that we are telling RA about all of the
crates in the workspace, but we are actually only telling it about a
subset). Consider the following dependency graph:
If we're invoked with a file from crate
A, we would tell RA aboutcrates
AandCwith the workspace manifest as the buildfile. If weare subsequently invoked with a file from crate
B, we'd still reportthe buildfile as the workspace manifest, but now we'd only tell RA about
crates
BandC. This causes RA to "unindex"A, since we're tellingRA that the workspace doesn't include it.
This commit rectifies the issue by reporting the current crate's
Cargo.toml as the buildfile in the
discoveroutput. However, thischanges the semantics of how and when
checkis invoked. In the aboveexample, we'd now report two separate "projects" after being invoked
with files from crates
AandB. BecauseAandBdo not depend oneach other, RA will invoke
checkseparately for each of them. This israther annoying if they share many dependencies (e.g. if
Chas a giantdependency tree), since those
checkinvocations will be duplicating alot of work.
This behavior can be adjusted by setting RA's
check.invocationStrategysetting to
"once", which tells it just to invoke check for the currentproject instead of all the projects we've told it about. This commit
also updates the README to reflect this change.
Fixes #34