-
Notifications
You must be signed in to change notification settings - Fork 22
Fix RSpec outlines #78
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
Conversation
|
Okay, yeah, this is no good. Each method definition has a body statement so this basically outlines every line of code in the file. 🙈 |
|
I guess we could just add the original query back. (call
method: (identifier) @run (#any-of? @run "describe" "context" "test" "it")
arguments: (argument_list . (_)+) @name
) @itemI think this would be compatible with the new changes. I don’t love that it will select these methods no matter where they appear in the tree. Like if I call At the moment, I’m struggling to even get the dev extension to build on |
6332c28 to
a0209d4
Compare
|
Okay, I’ve pushed up a change that I think will fix this issue, even though it’s much less precise. Ideally, there would be a way to select program and then any number of nested call blocks where the call message is one of Anyway, I can’t test this locally at the moment because I can’t get Zed to install a dev extension even on main. |
I guess we could manually do this for 3-4 levels of nesting. It’s unlikely many people will nest more than that. I just don’t think there’s a way to have an infinite repeating pattern unless I’m missing something. |
languages/ruby/outline.scm
Outdated
| )? | ||
| ) @item | ||
| ) | ||
| ; Test methods |
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 fixes the mentioned issue, but the query also captures one-line it blocks with empty contexts. For example, given this Ruby code:
describe 'content' do
it { is_expected.to_not allow_value('').for(:content) }
it { is_expected.to validate_length_of(:content).is_at_most(described_class::CONTENT_SIZE_LIMIT) }
end
The outline will be:
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.
We can do something simple and just assert that @run is not it:
; Test methods
(call
method: (identifier) @run @name (#any-of? @run "describe" "context" "test" "it")
arguments: (argument_list . [
(string) @name
(simple_symbol) @name
(scope_resolution) @name
(constant) @name
"," @context
]* [
(string) @name
(simple_symbol) @name
(scope_resolution) @name
(constant) @name
]
)?
(#not-eq? @run "it") ; <= Here
) @item
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.
But this would match something like this
def foo
bar.describe("test") do
# ...
end
endSupport up to 4 levels of describe/context/test/it nesting
|
I’ve updated this with 4 levels of nesting. Screen.Recording.2025-05-01.at.19.12.23.-.01.mp4 |
Thanks! It looks good, but I found some cases where the proposed outline queries don't capture certain it blocks. The |
|
I think that one is five levels deep, not 4. We'd have to add another level. It's in describe/describe/context/context/context/it. I didn't think people would nest that far. I can easily add another couple of levels. It will be slightly tricky to maintain since a lot of the query is duplicated. |
|
Okay, here's another idea that would allow for infinite nesting. We select the first level from program, either going via RSpec or a nil receiver. Then we look for it/test/describe/context nested in a describe/context block, but we don't ensure that it's describe/context blocks all the way from program. This wouldn't be perfect but it's unlikely you'd trigger it from non-test code. |
|
@vitallium I just updated the PR to use the technique I mentioned above. This seems to work with the file you linked. I also added |
Awesome, thank you! @joeldrapper I have a small suggestion. I think we can rely on tree-sitter' query precedence here and add support for one-liners like Thoughts? |
|
I'm concerned this might capture too much to |
|
@joeldrapper I think we can go ahead and land this. Thank you for working on it! |
Hi, this PR updates the Ruby extension to [v0.6.0](https://github.com/zed-extensions/ruby/releases/tag/v0.6.0). ## What's changed - Update Rust crate zed_extension_api to 0.5.0 by @renovate in zed-extensions/ruby#44 - Fix RSpec outlines by @joeldrapper in zed-extensions/ruby#78 Thanks!
Hi, this PR updates the Ruby extension to [v0.6.0](https://github.com/zed-extensions/ruby/releases/tag/v0.6.0). ## What's changed - Update Rust crate zed_extension_api to 0.5.0 by @renovate in zed-extensions/ruby#44 - Fix RSpec outlines by @joeldrapper in zed-extensions/ruby#78 Thanks!


Draft to fix #77
I need to verify there aren’t any unintended consequences of removing the outer
classandmoduleselectors.