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

Use Ripper.sexp instead of Regexp for completion #616

Closed
wants to merge 1 commit into from

Conversation

tompng
Copy link
Member

@tompng tompng commented Jun 27, 2023

Description

Replaces completion method from Regex based to Ripper.sexp base

Pros

Can complete these codes that Regexp based implementation provides wrong completion.

# Code  | Ripper | Regexp
{}.   #| Hash   | Hash or Proc
->{}. #| Proc   | Hash or Proc
%w{}. #| Array  | Hash or Proc
%{}.  #| String | Hash or Proc
[].   #| Array  | Array
a[].  #| ------ | Array
/a/i. #| Regexp | ------
%/a/. #| String | Regexp

Cons

Cannot complete if there are syntax error before cursor. Regexp based can complete these.

@;1.

1=1;1.

if a*(b+c)) < d
  1.

Limitation

Cannot complete these code now. I can support this later (about +40 lines).

# Inside ternary operator
a = condition ? true.a█ : false

# Inside block parameter
array.each_with_index do |value, index, other = 1.a█

Why not RubyVM::AbstractSyntaxTree or YARP

Using RubyVM::AbstractSyntaxTree (and I beleave YARP too) is easier than Ripper.sexp but:

  • RubyVM::AbstractSynatxTree: TruffleRuby does not have it
  • YARP: Not a gem now, ruby >= 3.1 is specified in Gemfile
  • Combination of both: Hard to maintain

Future plan

Use RBS to provide powerful completion

@st0012
Copy link
Member

st0012 commented Jun 27, 2023

I think using Ripper already caused some compatibility issues with implementations like TruffleRuby and JRuby? It's mostly why we still have tests pending for TruffleRuby (I paired with @nirvdrum on those).

Given that both TruffleRuby and JRuby are actively adopting YARP, which should support error recovery and has better API, how about:

  1. We make the current regexp completion a RegexpCompleter
  2. We build another completer called YARPCompleter and make it the default
  3. If the project uses Ruby 3.1-, we fallback to RegexpCompleter

@tompng
Copy link
Member Author

tompng commented Jun 28, 2023

Thank you for the feedback.

I think ripper compatibility issue is small. I found that pended test in test_ruby_lex and test_cmd passes.
#619

The reason of pend test failure was actually an incompatibility of SyntaxError#message, not Ripper.

irb/lib/irb/ruby-lex.rb

Lines 304 to 318 in 6269138

rescue SyntaxError => e
case e.message
when /unterminated (?:string|regexp) meets end of file/
# "unterminated regexp meets end of file"
#
# example:
# /
#
# "unterminated string meets end of file"
#
# example:
# '
return :recoverable_error
when /syntax error, unexpected end-of-input/
# "syntax error, unexpected end-of-input, expecting keyword_end"

(Before #500, test was failing but If I add some regexp pattern here, test passed)

RegexpCompleter and YARPCompleter idea looks good. I'll try it. I'm looking forward for YARP gem release.

@st0012
Copy link
Member

st0012 commented Oct 2, 2023

@tompng Since Prism (formerly YARP) has been released as a gem and I think the pros generally out-weights the cons, I'm fine with replacing RegexpCompletor completely with the completor we build with it (which also means we need to drop 2.7 support though).
But let's finish #707 first and implement the new completor with the specification we make there.

@tompng
Copy link
Member Author

tompng commented Oct 26, 2023

Closing this because of #708

@tompng tompng closed this Oct 26, 2023
@tompng tompng deleted the ripper_completion branch October 26, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants