-
Notifications
You must be signed in to change notification settings - Fork 112
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
Always use local variables in current context to parse code #397
Changes from 8 commits
5a4f06e
b823150
dcd8b62
5f582aa
da04c25
08c5575
7a064fa
aa38fa0
840f7b0
46589d6
1bd7434
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -506,13 +506,14 @@ def eval_input | |
|
||
@scanner.set_auto_indent(@context) if @context.auto_indent_mode | ||
|
||
@scanner.each_top_level_statement do |line, line_no| | ||
@scanner.each_top_level_statement(@context) do |line, line_no| | ||
signal_status(:IN_EVAL) do | ||
begin | ||
line.untaint if RUBY_VERSION < '2.7' | ||
if IRB.conf[:MEASURE] && IRB.conf[:MEASURE_CALLBACKS].empty? | ||
IRB.set_measure_callback | ||
end | ||
is_assignment = assignment_expression?(line) | ||
if IRB.conf[:MEASURE] && !IRB.conf[:MEASURE_CALLBACKS].empty? | ||
result = nil | ||
last_proc = proc{ result = @context.evaluate(line, line_no, exception: exc) } | ||
|
@@ -529,7 +530,7 @@ def eval_input | |
@context.evaluate(line, line_no, exception: exc) | ||
end | ||
if @context.echo? | ||
if assignment_expression?(line) | ||
if is_assignment | ||
if @context.echo_on_assignment? | ||
output_value(@context.echo_on_assignment? == :truncate) | ||
end | ||
|
@@ -827,9 +828,12 @@ def assignment_expression?(line) | |
# array of parsed expressions. The first element of each expression is the | ||
# expression's type. | ||
verbose, $VERBOSE = $VERBOSE, nil | ||
result = ASSIGNMENT_NODE_TYPES.include?(Ripper.sexp(line)&.dig(1,-1,0)) | ||
code = "#{RubyLex.generate_local_variables_assign_code(@context.local_variables) || 'nil;'}\n#{line}" | ||
# Get the last node_type of the line. drop(1) is to ignore the local_variables_assign_code part. | ||
node_type = Ripper.sexp(code)&.dig(1)&.drop(1)&.dig(-1, 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not super easy to understand. Would you mind adding a comment for it? |
||
ASSIGNMENT_NODE_TYPES.include?(node_type) | ||
ensure | ||
$VERBOSE = verbose | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel this should be in an |
||
result | ||
end | ||
|
||
ATTR_TTY = "\e[%sm" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -136,16 +136,18 @@ def set_prompt(p = nil, &block) | |
:on_param_error | ||
] | ||
|
||
def self.generate_local_variables_assign_code(local_variables) | ||
"#{local_variables.join('=')}=nil;" unless local_variables.empty? | ||
end | ||
|
||
def self.ripper_lex_without_warning(code, context: nil) | ||
verbose, $VERBOSE = $VERBOSE, nil | ||
if context | ||
lvars = context.workspace&.binding&.local_variables | ||
if lvars && !lvars.empty? | ||
code = "#{lvars.join('=')}=nil\n#{code}" | ||
line_no = 0 | ||
else | ||
line_no = 1 | ||
end | ||
lvars_code = generate_local_variables_assign_code(context&.local_variables || []) | ||
if lvars_code | ||
code = "#{lvars_code}\n#{code}" | ||
line_no = 0 | ||
else | ||
line_no = 1 | ||
end | ||
|
||
compile_with_errors_suppressed(code, line_no: line_no) do |inner_code, line_no| | ||
|
@@ -214,6 +216,8 @@ def check_state(code, tokens = nil, context: nil) | |
ltype = process_literal_type(tokens) | ||
indent = process_nesting_level(tokens) | ||
continue = process_continue(tokens) | ||
lvars_code = self.class.generate_local_variables_assign_code(context&.local_variables || []) | ||
code = "#{lvars_code}\n#{code}" if lvars_code | ||
code_block_open = check_code_block(code, tokens) | ||
[ltype, indent, continue, code_block_open] | ||
end | ||
|
@@ -233,13 +237,13 @@ def initialize_input | |
@code_block_open = false | ||
end | ||
|
||
def each_top_level_statement | ||
def each_top_level_statement(context) | ||
initialize_input | ||
catch(:TERM_INPUT) do | ||
loop do | ||
begin | ||
prompt | ||
unless l = lex | ||
unless l = lex(context) | ||
throw :TERM_INPUT if @line == '' | ||
else | ||
@line_no += l.count("\n") | ||
|
@@ -269,18 +273,15 @@ def each_top_level_statement | |
end | ||
end | ||
|
||
def lex | ||
def lex(context) | ||
line = @input.call | ||
if @io.respond_to?(:check_termination) | ||
return line # multiline | ||
end | ||
code = @line + (line.nil? ? '' : line) | ||
code.gsub!(/\s*\z/, '').concat("\n") | ||
@tokens = self.class.ripper_lex_without_warning(code) | ||
@continue = process_continue | ||
@code_block_open = check_code_block(code) | ||
@indent = process_nesting_level | ||
@ltype = process_literal_type | ||
@tokens = self.class.ripper_lex_without_warning(code, context: context) | ||
@ltype, @indent, @continue, @code_block_open = check_state(code, @tokens, context: context) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch on using |
||
line | ||
end | ||
|
||
|
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.
Why moving this line here though? It doesn't look like to change the behavior?
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.
It actually changes the behavior in a rare case.
assignment_expression?
check should be done before@context.evaluate
(in L519 and L530)I added test case for this
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.
Wow thanks for spotting this. I think it also worth a comment then?
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.
Thank you for the example 👍