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

Fix process_continue and check_code_block #611

Merged
merged 1 commit into from Jun 25, 2023

Conversation

tompng
Copy link
Member

@tompng tompng commented Jun 21, 2023

Rename and refactor process_continue and check_code_block

def process_continue(tokens)
  # doing something like maybe_should_continue?
  # check if last token is BEG (half part of `should_continue?`)
end

def check_code_block(code)
  # doing something like code_syntax_is_not_complete_or_maybe_should_continue?
  # check code is syntax_valid recoverable_error unrecoverable_error
  # check if the last token is DOT and checks other duplicated condition with syntax check and open_tokens check (other half part of `should_continue?`)
end

def should_continue?(tokens) = true or false
def check_code_syntax(code) = :valid or :recoverable_error or :unrecoverable_error or :other_error

def code_terminated?(code, tokens, opens)
  # calculate by should_continue?() and check_code_syntax()
end

This will fix prompt bug introduced in #500 or #515. (because process_continue was incomplete, half of the necessary implementation was in check_code_block)

# actual
irb(main):001:1> if 1
irb(main):002:0> end.

# expected
irb(main):001:1> if 1
irb(main):002:0* end.

Other fix

Previous implementation of process_continue only uses last two tokens.
It ignores only the last token newline tokens
I change it to ignore consecutive trailing comments, spaces, newlines.
It will fix this old bug.

actual
irb(main):001:0* 1; # comment
irb(main):002:0* 
irb(main):003:0* 

expected
irb(main):001:0> 1; # comment
=> 1
irb(main):002:0> 

@tompng tompng force-pushed the process_continue_fix branch 4 times, most recently from c344fd0 to 6a1d537 Compare June 22, 2023 20:28
@tompng tompng marked this pull request as ready for review June 22, 2023 20:36
@st0012 st0012 added the bug Something isn't working label Jun 24, 2023
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.

LGTM 👍 Just need to fix some conflicts

@tompng tompng merged commit b7f4bfa into ruby:master Jun 25, 2023
24 checks passed
@tompng tompng deleted the process_continue_fix branch June 25, 2023 05:12
matzbot pushed a commit to ruby/ruby that referenced this pull request Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

2 participants