Skip to content

Remove rubyvm: keyword argument from Prism.find - #4202

Open
eregon wants to merge 1 commit into
ruby:mainfrom
eregon:find-remove-rubyvm-keyword-argument
Open

Remove rubyvm: keyword argument from Prism.find#4202
eregon wants to merge 1 commit into
ruby:mainfrom
eregon:find-remove-rubyvm-keyword-argument

Conversation

@eregon

@eregon eregon commented Aug 6, 2026

Copy link
Copy Markdown
Member

Remove the rubyvm: keyword argument from Prism.find. The internal dispatch already checks defined?(::RubyVM), so exposing this as a public parameter seems unnecessary. Removing it makes the API cleaner and makes it straightforward to test each Find implementation directly using the concrete classes.

Additionally, unified LineMethodFind, LineLambdaFind, and LineProcFind into a single LineCallableFind class, since they share the same structure (parse file, match by start line).

@eregon
eregon requested review from Earlopain and kddnewton August 6, 2026 20:20
@eregon

eregon commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

(CI seems to not run because of a GitHub Actions outage)

@eregon
eregon force-pushed the find-remove-rubyvm-keyword-argument branch from 80cf417 to fe57f42 Compare August 7, 2026 15:32
Comment thread lib/prism/node_find.rb
else
LineProcFind.new.find(callable)
end
when Method, UnboundMethod

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer these to stay as separate classes, as the point is that you only have to switch on the callable in one place.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. It also makes the diff a lot smaller.

* This does not scale well with more find implementations.
* RubyVM is CRuby-specific and experimental so not something usually exposed in public APIs.
* Tests can use the specific class directly, which is more reliable.
@eregon
eregon force-pushed the find-remove-rubyvm-keyword-argument branch from fe57f42 to 0359415 Compare August 14, 2026 19:31
@eregon
eregon requested a review from kddnewton August 14, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants