Skip to content

Conversation

@pocke
Copy link
Member

@pocke pocke commented Jul 8, 2021

Update

This PR focus on paren only. The following description is obsolate.


This pull request fixes a problem that this gem doesn't work if parens, comments, and/or backslashes exist after the receiver.

It is under WIP because I need to support other nodes such as OPCALL.

Problem

This gem doesn't display ^^^ if any non-space characters that don't make node exist. For example:

(42).time

42
  # comment
  .time

42 \
  .time

The syntaxes don't make RubyVM::AST node, so nd_recv.last_column doesn't mean the position Immediately before the dot.

@pocke pocke force-pushed the Keep_it_work_if_paren__comment_and_or_backslashes_exist_after_receiver branch from 4ad0bea to 8303491 Compare July 8, 2021 11:49
@ujihisa
Copy link

ujihisa commented Jul 8, 2021

(Another example:

<<"this is a pen"; p 42
this is a pen
  .times

)

@mame
Copy link
Member

mame commented Jul 8, 2021

Thank you for the report. I think it is enough to care only parens.

Actually I was aware that it does not work well when there is a comment or a backslash between the receiver and ., but I believe that such code is rare. Even in such a case, this gem shows no snippet, which is not a big deal, IMO. But I agree that it is good to support (42).time.

@pocke
Copy link
Member Author

pocke commented Jul 9, 2021

Thanks for your replies.

I agree with @mame. The regexps will be too complicated but there are only a few benefits. If we need to support them, it should be solved with RubyVM::AST improvements, such as node.pos(:method_name).first_column, but not regexp.

I'll update this pull request to only support (42).time case.


Just FYI, there is the same problem with multiline comment between . and method name. (and It is a super super rare case!)

42.
=begin
comment
=end
  time

@pocke pocke force-pushed the Keep_it_work_if_paren__comment_and_or_backslashes_exist_after_receiver branch from 8303491 to 4acae4e Compare July 9, 2021 12:30
@pocke pocke changed the title WIP Keep it work if paren, comment and/or backslashes exist after receiver WIP Keep it work if paren exists after receiver Jul 9, 2021
@pocke pocke changed the title WIP Keep it work if paren exists after receiver Keep it work if paren exists after receiver Jul 9, 2021
@pocke pocke marked this pull request as ready for review July 9, 2021 12:30
@pocke
Copy link
Member Author

pocke commented Jul 9, 2021

I've updated this pr to support only parens, and updated for all nodes that were broken on parens.

@mame mame merged commit b79d679 into ruby:master Jul 31, 2021
@pocke pocke deleted the Keep_it_work_if_paren__comment_and_or_backslashes_exist_after_receiver branch July 31, 2021 13:15
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.

3 participants