Skip to content

Remove mk_sp_lo_plus_one function. #4887

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

Closed
wants to merge 1 commit into from
Closed

Conversation

crlf0710
Copy link
Member

@crlf0710 crlf0710 commented Jul 3, 2021

This function is a footgun to usage since lo+1 might not reside on a char boundary.

Previous before #4880 there're only two usages of this function. This pr removes the only remaining usage along-side with it.

@calebcartwright calebcartwright changed the base branch from master to stbu July 30, 2021 02:25
@calebcartwright
Copy link
Member

Thank you for the PR but I'm going to close this, both because I disagree with the characterization and more generally because it's a change going in the opposite direction of where we need to go.

We really need to make more utility functions available to replace the myriad span derivations happening with bytepos throughout the codebase, including leveraging more of the functions directly provided by the source map (e.g. next_point) that properly handle multibyte characters when we're dealing with span boundaries.

Dropping utility functions is only going to shift such problems out to the corresponding call sites, and I'd much rather have things encapsulated as much as possible

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