Use runnable kind to disambiguate between cargo and shell commands#22295
Merged
Veykril merged 1 commit intorust-lang:masterfrom May 8, 2026
Merged
Use runnable kind to disambiguate between cargo and shell commands#22295Veykril merged 1 commit intorust-lang:masterfrom
Veykril merged 1 commit intorust-lang:masterfrom
Conversation
rustbot
added
the
S-waiting-on-review
Contributor
Author
|
I'm wondering though if r-a should just drop the |
sunshowers
force-pushed
the
runnable-kind
branch
2 times, most recently
from
c3b2616 to
65668d9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
rustbot
removed
the
S-waiting-on-review
Found this bug while investigating why configuring nextest based on the instructions at rust-lang#21137 (comment) wasn't working within Zed. Previously, we'd use `serde(untagged)`, preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command: ```json { "label": "test my_test", "kind": "shell", "args": { "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"}, "cwd": "/project", "program": "cargo", "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"] } } ``` would end up getting deserialized as a Cargo command, silently dropping `program` and `args`. With this fix, we now use the provided `kind` as a tag. We do have to introduce a `#[serde(flatten)]` unfortunately, which has a few side effects due to internal buffering, but `#[serde(untagged)]` also does internal buffering so this doesn't make things worse. See zed-industries/zed#54011 for the original fix to Zed.
rustbot
added
the
S-waiting-on-review
Member
I think restricting to tests would make sense. I don't think r-a would ever need to deserialize these. |
rustbot
removed
the
S-waiting-on-review
Member
|
Turns out we can't test cfg these due to the Request trait requirements on its assoc types |
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
3 participants
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.
See zed-industries/zed#54011 for the original fix to Zed. That project had copied over these type definitions from rust-analyzer, so we should fix that here too, even though I don't believe r-a deserializes these in production.
Found this bug while investigating why configuring nextest based on the instructions at #21137 (comment) wasn't working within Zed.
Previously, we'd use
serde(untagged), preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:{ "label": "test my_test", "kind": "shell", "args": { "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"}, "cwd": "/project", "program": "cargo", "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"] } }would end up getting deserialized as a Cargo command, silently dropping
programandargs.With this fix, we now use the provided
kindas a tag. We do have to introduce a#[serde(flatten)]unfortunately, which has a few side effects due to internal buffering, but#[serde(untagged)]also does internal buffering so this doesn't make things worse.