- unify locations for normal and endless method definition #718
Conversation
|
Note: this keeps locations untouched for |
| loc(end_t)) | ||
| Source::Map::MethodDefinition.new(loc(keyword_t), | ||
| loc(operator_t), loc(name_t), | ||
| loc(end_t), nil, nil) | ||
| end | ||
|
|
||
| def endless_definition_map(keyword_t, operator_t, name_t, assignment_t, body_e) |
There was a problem hiding this comment.
let's rename this method to method_definition_map
There was a problem hiding this comment.
Sure. But we'll need to keep an alias for compatibility, right?
There was a problem hiding this comment.
endless_definition_map is (was) used only in ruby28.y that is still in development. I think it's ok to rename without any aliases
There was a problem hiding this comment.
Oh, sorry, I thought you wanted definition_map => method_definition_map.
Would you like to rename endless_definition_map => method_definition_map and that this is used for standard and endless method definitions?
My PR uses definition_map for standard methods, and endless_definition_map for endless ones. This way definition_map remains as a valid API and the implementation is updated, but maybe there's a better way to go.
There was a problem hiding this comment.
(seems it would be confusing if standard methods used definition_map and endless ones used method_definition_map)
There was a problem hiding this comment.
nvm, I think it's better to keep it as is.
EDIT: just got a notification from GH that you reverted it. Yes, let's keep it as is, otherwise it sounds like it handles all method definitions. 👍
There was a problem hiding this comment.
Yes, I had misunderstood your comment (although you clearly made it on the def endless_definition_map, I should have paid more attention)
marcandre
force-pushed
the
endless_loc
branch
2 times, most recently
from
ca95288
to
6826c1a
Compare
iliabylich
changed the title
| @@ -1856,6 +1856,7 @@ def test_def | |||
| %q{def foo; end}, | |||
| %q{~~~ keyword | |||
| | ~~~ name | |||
| |! assignment | |||
There was a problem hiding this comment.
@iliabylich I'm glad you suggested we introduce this notation, didn't think I'd use it so soon 🙇♂️
|
@marcandre Do you need a release? |
|
Not really, but maybe @palkan would like one (he pointed this out) |
|
No rush, I'm good, thanks) Just want to make sure the tests pass against master. |
This way one can ask
loc.endorloc.assignmentof all method definitions