-
Notifications
You must be signed in to change notification settings - Fork 121
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
Split TypeCompletion to a gem #772
Conversation
2fff82e
to
c8c00c7
Compare
lib/irb/completion.rb
Outdated
def initialize(source_file = nil) | ||
require 'repl_type_completor' | ||
ReplTypeCompletor.preload_rbs | ||
@source_file = source_file |
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.
Does this mean in the entire IRB session, user only gets the completion targeting one source file? For example, if I put a binding.irb
under app/models/user.rb
, will it run all analysis only around user.rb
?
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.
Nice catch. Actually, this initialization (build_type_completor and also IRB::Irb.new
) will be called in each binding.irb
.
But I found that @irb_path
is not fully initialized yet when build_completor is called and context.irb_path
is overwritten after IRB::Irb
initialization.
# In irb.rb:1050
binding_irb = IRB::Irb.new(workspace)
binding_irb.context.irb_path = irb_path
binding_irb.run(IRB.conf)
I will change to IRB::TypeCompletor.new(context)
and call ReplTypeCompletor.analyze(preposing + target, binding: bind, filename: context.irb_path)
on completion phase.
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.
Yeah I think that's better because with irb:rdbg
, irb_path
could be updated if user moves around with the debugger. If we store it as a state in InputMethod, it won't be synced correctly.
c8c00c7
to
91c6e37
Compare
91c6e37
to
6aaf807
Compare
@@ -13,7 +13,7 @@ desc "Run each irb test file in isolation." | |||
task :test_in_isolation do | |||
failed = false | |||
|
|||
FileList["test/irb/test_*.rb", "test/irb/type_completion/test_*.rb"].each do |test_file| | |||
FileList["test/irb/**/test_*.rb"].each do |test_file| |
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.
Ah I forgot to change this in #794 🤦♂️
Description
TypeCompletion requires Prism and RBS. Since IRB is a default gem and IRB supports ruby>=2.7.0, we can't write Prism dependency(ruby>=3.0) to gemspec.
Splitting to gem allows to simplify the version checks and other. (Other problems are described in #734 (comment))
API of repl_type_completor
https://github.com/ruby/repl_type_completor/blob/main/sig/repl_type_completor.rbs
Future extending plans
The
completion_candidates
anddoc_namespace
is insufficient when we want toabort (method)
alias (keyword)
and (keyword)
at_exit (method)
a_local_var (lvar)
)We can add a method to
ReplCompletion::Result
to extend.Duplicate implementation with IRB
Gem repl_type_completor has a reimplementation of
IRB::InputCompletor#retrieve_files_to_require_from_load_path
andIRB::InputCompletor#retrieve_files_to_require_relative_from_current_dir
.It is duplicated but I think it should be calculated in the separated gem.
Also IRB can't delete these methods until dropping RegexpCompletor.
filename:
optionWe need source file path to correctly complete require_relative paths.
require_relative
will load relative path from__dir__
, not fromDir.pwd
.IRB.CurrentContext.irb_path
is passed toeval(string, binding, filename, lineno)
and completor needs this value.