Skip to content
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

WIP: Add support for detecting tests in source files, and implement it for Rust #10873

Closed
wants to merge 15 commits into from

Conversation

mikayla-maki
Copy link
Contributor

@mikayla-maki mikayla-maki commented Apr 23, 2024

TODO:

  • Wire up a new query
  • Use this query to find tests in the current file range
  • Render a play button in that location
  • Spawn the test template from that play button when clicked

Release Notes:

  • TODO

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Apr 23, 2024
@mikayla-maki mikayla-maki marked this pull request as draft April 23, 2024 00:40
@mikayla-maki mikayla-maki self-assigned this Apr 23, 2024
@osiewicz
Copy link
Contributor

A question/food for thought: will all of the tests show up in a task modal?
On one hand I think that'll bloat the modal, but on the other it would be inconsistent to not include them.

@davidbarsky
Copy link

Hi! Sorry for butting in (asking since I'm a maintainer on rust-analyzer and I use Zed...), but would it be possible to use rust-analyzer's runnable functionality instead, by any chance?

(We'll be changing to extend support for non-Cargo build systems in rust-lang/rust-analyzer#16840, but we don't expect to change the custom LSP request beyond what's in that PR.)

@osiewicz
Copy link
Contributor

Hey,
We did consider using an lsp extension for tasks (see #7108), but it does not solve test discoverability problem for language servers without these kinds of protocol extensions. In theory though we can consume VSC tasks just fine.

@davidbarsky
Copy link

Hey,

We did consider using an lsp extension for tasks (see #7108), but it does not solve test discoverability problem for language servers without these kinds of protocol extensions. In theory though we can consume VSC tasks just fine.

Thanks! I read through that, I appreciate you linking to your thinking on the matter! Obviously, y'all are the maintainers, but I'm curious as to how feasible it'd be to take a treesitter-based approach for all languages as a baseline and enhance specific languages (such as Rust) with LSP extensions. The benefit of LSP-based tasks would be more accurate task generation because rust-analyzer is able to see through macro expansions and whatnot.

From an implementation standpoint, I think there might be some complications around scaling this approach/figuring out how it interacts with Zed's extension API, but for rust-analyzer at least, I'd be happy to send PRs to Zed each time that request corresponding to runnables changes (the PR I linked to would be first change in 3-4 years, fwiw).

I don't expect much would change on the Zed side of things, since I think the task templates y'all have are sufficiently flexible. I think the integration boils down to mapping the right fields in the runnable response to the pre-existing task template in Zed.

@mikayla-maki
Copy link
Contributor Author

mikayla-maki commented Apr 24, 2024

but I'm curious as to how feasible it'd be to take a treesitter-based approach for all languages as a baseline and enhance specific languages (such as Rust) with LSP extensions.

That's where I'm thinking we'll end up :D We'd love support on maintaining experimental API protocols like this :) @davidbarsky

@davidbarsky
Copy link

but I'm curious as to how feasible it'd be to take a treesitter-based approach for all languages as a baseline and enhance specific languages (such as Rust) with LSP extensions.

That's where I'm thinking we'll end up :D We'd love support on maintaining experimental API protocols like this :) @davidbarsky

That's great to hear! I wasn't sure how open y'all would be to that, but I'm very happy you are! I'll keep my eyes peeled on opportunities for changes that make that possible.

@osiewicz
Copy link
Contributor

Closing this in favor of #11195

@osiewicz osiewicz closed this Apr 30, 2024
osiewicz added a commit that referenced this pull request May 5, 2024
… Rust (#11195)

Continuing work from #10873 

Release Notes:

- N/A

---------

Co-authored-by: Mikayla <mikayla@zed.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants