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

fix(Range): rename locate to located #158

Closed
wants to merge 1 commit into from
Closed

fix(Range): rename locate to located #158

wants to merge 1 commit into from

Conversation

favonia
Copy link
Contributor

@favonia favonia commented Oct 7, 2024

While finalizing the debugging interface, I noticed that the Range.locate is taking a location to create an element of located, not locating an element. I'd like to fix this API naming.

Old name New name Typical usage
Range.locate Range.located located loc term
Range.locate_opt Range.located_opt located_opt loc term where loc can be None
Range.locate_lex Range.located_lex located_lex $loc term (in Menhir)

I'm not sure if located_lex is the best name, but locate_lex seems wrong.

@mikeshulman
Copy link
Collaborator

Are you saying that "giving a location to an element" shouldn't be described as "locating an element"?

@favonia
Copy link
Contributor Author

favonia commented Oct 7, 2024

@mikeshulman I thought "locate" means "finding the location of something when we don't know where it is", but in this case the location is explicitly given. I'm not insistent on this PR if native speakers feel otherwise.

@favonia favonia force-pushed the range-located branch 2 times, most recently from 3bc8bcc to 9f89ea3 Compare October 7, 2024 21:39
@mikeshulman
Copy link
Collaborator

In Merriam-Webster, the first definition of the transitive "locate" is "to determine or indicate the place, site, or limits of", but the second is "to set or establish in a particular spot". So I think this use of "locate" is justifiable.

@favonia
Copy link
Contributor Author

favonia commented Oct 8, 2024

@mikeshulman Got it. Abandoning this PR now.

@favonia favonia closed this Oct 8, 2024
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