Skip to content

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