Skip to content
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

+ Add associate_by_identity as an alternate to associate #798

Merged
merged 1 commit into from May 2, 2021

Conversation

marcandre
Copy link
Contributor

In #184, it was noticed that associate's result is unusable for separate nodes that might have identical content.

The method associate_by_locations, which uses locations instead of nodes as indices, was added as a workaround.

It was discussed at the time to change the definition of equality for Node (which was rejected) but it doesn't look like using Hash#compare_by_identity was discussed which surprises me. For any user of parser that does not play around its Nodes and only uses those returned from parsing, this seems like the perfect solution.

This PR proposes to add associate_by_identity that does just that.

This would be helpful for rubocop/rubocop#9756

If this is accepted, a release would be appreciated.

@iliabylich iliabylich changed the title Add associate_by_identity as an alternate to associate + Add associate_by_identity as an alternate to associate May 2, 2021
@iliabylich iliabylich merged commit edd0e47 into whitequark:master May 2, 2021
@iliabylich
Copy link
Collaborator

@marcandre Thanks, 3.0.1.1 has been released.

@marcandre marcandre deleted the associate_by_identity branch May 2, 2021 09:57
@marcandre
Copy link
Contributor Author

🎉 Thank you @iliabylich for being so awesome ❤️

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.

None yet

2 participants