Skip to content

Conversation

@tompng
Copy link
Member

@tompng tompng commented Apr 11, 2025

#1024
Migrate nesting analysis from Ripper to Prism

Original nesting calculation with Ripper

Tokenize with ripper
Parse the tokens mainly focusing on open/close tokens
Collect open tokens per line

New nesting calculation with Prism

Traverse syntax tree
Check open_loc and closing_loc for a node that makes nesting, create a token-like object for it
Collect open token-like objects per line

Gemfile

Exclude prism-1.8.0 because test fails. (ruby/prism#3851)

@tompng tompng mentioned this pull request Jan 15, 2026
@tompng tompng force-pushed the ripper_to_prism_nesting branch 2 times, most recently from 4c8c4f2 to be7dfe6 Compare January 16, 2026 16:43
@tompng tompng force-pushed the ripper_to_prism_nesting branch from be7dfe6 to c3a31b8 Compare January 16, 2026 16:52
@tompng tompng marked this pull request as ready for review January 16, 2026 17:28
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Can we nudge Prism maintainers to cut v1.8.1 for the fix, and add prism as a dependency and require 1.3.0+?

# scan_opens without block will return a list of open tokens at last token position
scan_opens(tokens)
# Return a list of open nestings at last token position
def open_nestings(parse_lex_result)
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this or parse_by_line take (code, local_variables: []) and call Prism.parse_lex here?
I feel Prism should be hidden from the callers of this class. Then we only need to require it here instead of all the places that'd use a nesting parser.

Copy link
Member Author

Choose a reason for hiding this comment

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

In a followup pull request #1160, parse_lex_result will be also used in RubyLex#should_continue?.
To avoid parsing the same code two times in dynamic_prompt calculation, argument needs to be parse_lex_result.

@context.io.dynamic_prompt do |lines|
  parse_lex_result = Prism.parse_lex(code, scopes: [@context.local_variables])
  line_results = IRB::NestingParser.parse_by_line(parse_lex_result)
  ...
  line_results.map.with_index do |...|
    # This part requires lex result
    continue = @scanner.should_continue?(tokens_until_line, line, line_num_offset + 1)
    ...
  end
end

Maybe we can find a better way to separate after migrating to Prism, I think.

@st0012
Copy link
Member

st0012 commented Jan 16, 2026

Oh you did this in #1091 already. I guess we just need to wait for the release then

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