-
Notifications
You must be signed in to change notification settings - Fork 146
Highlight the method name in method calls #1189
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
Changes from 6 commits
061f494
9c9f1f6
d6369b9
66933a9
8643c13
0b398c3
db9cd4c
dd0469a
d9ef6d9
89f5bae
2c7df4d
fa97fdd
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 | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -103,6 +103,7 @@ module Color | |||||||||||||||||||||||||
| __END__: [GREEN], | ||||||||||||||||||||||||||
| # tokens from syntax tree traversal | ||||||||||||||||||||||||||
| method_name: [BLUE, BOLD], | ||||||||||||||||||||||||||
| message_name: [BLUE], | ||||||||||||||||||||||||||
| symbol: [YELLOW], | ||||||||||||||||||||||||||
| # special colorization | ||||||||||||||||||||||||||
| error: [RED, REVERSE], | ||||||||||||||||||||||||||
|
|
@@ -111,7 +112,11 @@ module Color | |||||||||||||||||||||||||
| styles.map { |style| "\e[#{style}m" }.join | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
| CLEAR_SEQ = "\e[#{CLEAR}m" | ||||||||||||||||||||||||||
| private_constant :TOKEN_SEQS, :CLEAR_SEQ | ||||||||||||||||||||||||||
| OPERATORS = [ | ||||||||||||||||||||||||||
| :!=, :!~, :=~, :==, :===, :<=>, :>, :>=, :<, :<=, :&, :|, :^, :>>, :<<, :-, :+, :%, :/, :*, :**, | ||||||||||||||||||||||||||
| :-@, :+@, :~, :!, :[], :[]= | ||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||
| private_constant :TOKEN_SEQS, :CLEAR_SEQ, :OPERATORS | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| class << self | ||||||||||||||||||||||||||
| def colorable? | ||||||||||||||||||||||||||
|
|
@@ -160,7 +165,7 @@ def colorize(text, seq, colorable: colorable?) | |||||||||||||||||||||||||
| # If `complete` is false (code is incomplete), this does not warn compile_error. | ||||||||||||||||||||||||||
| # This option is needed to avoid warning a user when the compile_error is happening | ||||||||||||||||||||||||||
| # because the input is not wrong but just incomplete. | ||||||||||||||||||||||||||
| def colorize_code(code, complete: true, ignore_error: false, colorable: colorable?, local_variables: []) | ||||||||||||||||||||||||||
| def colorize_code(code, complete: true, ignore_error: false, colorable: colorable?, colorize_call: true, local_variables: []) | ||||||||||||||||||||||||||
| return code unless colorable | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| result = Prism.parse_lex(code, scopes: [local_variables]) | ||||||||||||||||||||||||||
|
|
@@ -175,7 +180,7 @@ def colorize_code(code, complete: true, ignore_error: false, colorable: colorabl | |||||||||||||||||||||||||
| errors = filter_incomplete_code_errors(errors, prism_tokens) | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| visitor = ColorizeVisitor.new | ||||||||||||||||||||||||||
| visitor = ColorizeVisitor.new(colorize_call: colorize_call) | ||||||||||||||||||||||||||
| prism_node.accept(visitor) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| error_tokens = errors.map { |e| [e.location.start_line, e.location.start_column, 0, e.location.end_line, e.location.end_column, :error, e.location.slice] } | ||||||||||||||||||||||||||
|
|
@@ -229,7 +234,8 @@ def colorize_code(code, complete: true, ignore_error: false, colorable: colorabl | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| class ColorizeVisitor < Prism::Visitor | ||||||||||||||||||||||||||
| attr_reader :tokens | ||||||||||||||||||||||||||
| def initialize | ||||||||||||||||||||||||||
| def initialize(colorize_call: true) | ||||||||||||||||||||||||||
| @colorize_call = colorize_call | ||||||||||||||||||||||||||
| @tokens = [] | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -252,6 +258,26 @@ def visit_def_node(node) | |||||||||||||||||||||||||
| super | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def visit_call_node(node) | ||||||||||||||||||||||||||
| colorize_call(node) | ||||||||||||||||||||||||||
| super | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def visit_call_operator_write_node(node) | ||||||||||||||||||||||||||
| colorize_call(node) | ||||||||||||||||||||||||||
| super | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def visit_call_and_write_node(node) | ||||||||||||||||||||||||||
| colorize_call(node) | ||||||||||||||||||||||||||
| super | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def visit_call_or_write_node(node) | ||||||||||||||||||||||||||
| colorize_call(node) | ||||||||||||||||||||||||||
| super | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def visit_interpolated_symbol_node(node) | ||||||||||||||||||||||||||
| dispatch node.opening_loc, :symbol | ||||||||||||||||||||||||||
| node.parts.each do |part| | ||||||||||||||||||||||||||
|
|
@@ -279,6 +305,21 @@ def visit_symbol_node(node) | |||||||||||||||||||||||||
| dispatch node.closing_loc, :symbol | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def colorize_call(node) | ||||||||||||||||||||||||||
| if @colorize_call | ||||||||||||||||||||||||||
| if node.call_operator_loc.nil? && OPERATORS.include?(node.name) | ||||||||||||||||||||||||||
| # Operators should not be colored as method call | ||||||||||||||||||||||||||
| elsif (node.call_operator_loc.nil? || node.call_operator_loc.slice == "::") && | ||||||||||||||||||||||||||
| /\A\p{Upper}/.match?(node.name) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| if node.call_operator_loc.nil? && OPERATORS.include?(node.name) | |
| # Operators should not be colored as method call | |
| elsif (node.call_operator_loc.nil? || node.call_operator_loc.slice == "::") && | |
| /\A\p{Upper}/.match?(node.name) | |
| name = node.name | |
| name_sym = name.is_a?(Symbol) ? name : name.to_sym | |
| name_str = name.to_s | |
| if node.call_operator_loc.nil? && OPERATORS.include?(name_sym) | |
| # Operators should not be colored as method call | |
| elsif (node.call_operator_loc.nil? || node.call_operator_loc.slice == "::") && | |
| /\A\p{Upper}/.match?(name_str) |
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.
Besides that, CallOperatorWriteNode seems not to have the name method. I'll fix it.
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.
Fixed in db9cd4c.
Regexp#match? accepts symbols, so there's no need to call to_s.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ def text(str, width = nil) | |
| when /\A#</, '=', '>' | ||
| super(@colorize ? Color.colorize(str, [:GREEN]) : str, width) | ||
| else | ||
| super(@colorize ? Color.colorize_code(str, ignore_error: true) : str, width) | ||
| super(@colorize ? Color.colorize_code(str, ignore_error: true, colorize_call: false) : str, width) | ||
|
Member
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. Why do we not want to colorize call in this case?
Member
Author
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. The above code was introduced to fix the following failure of the debug compatibility test: https://github.com/ruby/irb/actions/runs/23522320220/job/68468010147
Member
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. Hmm. I don't like that we update IRB lib code to workaround a test. I'd prefer:
An alternative could be to disable this test until 2 is merged. @tompng WDYT?
Member
Author
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'm not sure if it's correct to highlight member names as method names in the Struct inspect output. Furthermore, the results might differ in environments where variables with the same names as the members are defined.
Member
Author
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 might be an overthought, as no local variables are specified here.
Member
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. Ship without this workaround and fix debug test seems good 👍
Member
Author
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've reverted it in fa97fdd. |
||
| end | ||
| end | ||
| 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.
Writer methods are also colored blue, but Writer methods with operators are not colored.
So I think we can also add visitor methods for CallAndWriteNode CallOperatorWriteNode CallOrWriteNode.
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.
I've fixed it in 66933a9.