Skip to content

Rename some keyword locations for consistency#4060

Open
Earlopain wants to merge 1 commit intoruby:mainfrom
Earlopain:consistent-keyword-locs
Open

Rename some keyword locations for consistency#4060
Earlopain wants to merge 1 commit intoruby:mainfrom
Earlopain:consistent-keyword-locs

Conversation

@Earlopain
Copy link
Copy Markdown
Collaborator

Prism is generally good about their naming but these had some inconsistencies compared to others:

  • InNode was missing the keyword in the name
  • MatchPredicateNode in was named operator instead
  • UnlessNode/UntilNode/WhenNode/WhileNode has more than one keyword, with one generic keyword_loc
  • UntilNode/WhileNode named their end keyword closing_loc

Left the old names around for compatibility

Prism is generally good about their naming but these had some inconsistencies compared to others:

* `InNode` was missing the `keyword` in the name
* `MatchPredicateNode` `in` was named `operator` instead
* `UnlessNode`/`UntilNode`/`WhenNode`/`WhileNode` has more than one keyword, with one generic `keyword_loc`
* `UntilNode`/`WhileNode` named their `end` keyword `closing_loc`

Left the old names around for compatibility
@kddnewton
Copy link
Copy Markdown
Collaborator

I'm generally okay with doing this, but I just want to check — can you enumerate our rules for naming? Maybe we should explicitly write them down? Is it keyword_loc if there's only one and then *_keyword_loc when there are multiple?

@Earlopain
Copy link
Copy Markdown
Collaborator Author

Yeah, you got that right basically.

Some like {/do for blocks are just opening since it's not exclusively for keywords.

IfNode is a bit messy since it's for both keyword and ternary and it reuses the keyword locations for :/? with ElseNode. Conceptually they are very close but structure is pretty different. Seems like that should maybe have been a separate node but I don't want to touch that.

I don't really know what where to put docs about general rules, is there a place that would fit? I can write something if you want (although I don't think it would be relevant very often, should just look at rdoc)

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.

2 participants