-
Notifications
You must be signed in to change notification settings - Fork 143
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
base: master
Are you sure you want to change the base?
Changes from all commits
061f494
9c9f1f6
d6369b9
66933a9
8643c13
0b398c3
db9cd4c
dd0469a
d9ef6d9
89f5bae
2c7df4d
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 |
|---|---|---|
|
|
@@ -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 👍 |
||
| 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.