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

Split TypeCompletion to a gem #772

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Conversation

tompng
Copy link
Member

@tompng tompng commented Nov 20, 2023

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

module ReplTypeCompletor
  VERSION: String

  class Result
    def completion_candidates: () -> Array[String]
    def doc_namespace: (String) -> String?
  end

  def self.analyze: (String, binding: Binding, ?filename: String?) -> Result?

  def self.rbs_load_error: () -> Exception?
  def self.last_completion_error: () -> Exception?

  def self.rbs_load_started?: () -> bool
  def self.rbs_loaded?: () -> bool
  def self.load_rbs: () -> void
  def self.preload_rbs: () -> void
  def self.info: () -> String
end

Future extending plans

The completion_candidates and doc_namespace is insufficient when we want to

  • Show a hint in completion dialog (example: abort (method) alias (keyword) and (keyword) at_exit (method) a_local_var (lvar))
  • Show something in document dialog (example: RBS signature) when there is no document
  • Add another dialog to show something (example: possible types of receiver in method call)

We can add a method to ReplCompletion::Result to extend.

# These are just an idea

result.categorized_candidates
#=>
[
  { type: :lvar, candidates: ['abc, 'abcde'] },
  { type: :keyword, candidates: ['and', 'alias'] },
  { type: :call, candidates: ['abort', 'at_exit'], receiver: Kernel },
  { type: :call, candidates: ['aaa', 'aab'], receiver: main.singleton_class },
]

result.match_info(matched)
# =>
{
  type: :lvar,
  value_classes: [Array, Hash],
  value_type_inspect: 'Array[String] | Hash[Symbol, String | Integer]',
  other_useful_information: something
}

Duplicate implementation with IRB

Gem repl_type_completor has a reimplementation of IRB::InputCompletor#retrieve_files_to_require_from_load_path and IRB::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: option

We need source file path to correctly complete require_relative paths.
require_relative will load relative path from __dir__, not from Dir.pwd.
IRB.CurrentContext.irb_path is passed to eval(string, binding, filename, lineno) and completor needs this value.

# Using binding.source_locaiton[0] as a filename is not sufficient because:
irb(main):001> IRB.CurrentContext.workspace.binding.source_location[0]
=> "/path/to/irb/lib/irb/workspace.rb" # wrong base path for require_relative
irb(main):002> __FILE__
=> "(irb)" # In this case, looks like `Dir.pwd` is used instead

@tompng tompng force-pushed the use_repl_completion_gem branch 4 times, most recently from 2fff82e to c8c00c7 Compare November 28, 2023 19:13
@tompng tompng marked this pull request as ready for review November 28, 2023 19:17
lib/irb/context.rb Outdated Show resolved Hide resolved
test/irb/test_context.rb Outdated Show resolved Hide resolved
def initialize(source_file = nil)
require 'repl_type_completor'
ReplTypeCompletor.preload_rbs
@source_file = source_file
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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.

@@ -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|
Copy link
Member

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 🤦‍♂️

@st0012 st0012 added the enhancement New feature or request label Nov 29, 2023
@tompng tompng merged commit a4868a5 into ruby:master Nov 29, 2023
22 of 24 checks passed
@tompng tompng deleted the use_repl_completion_gem branch November 29, 2023 16:30
matzbot pushed a commit to ruby/ruby that referenced this pull request Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants