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

Error on $<num><non_num> capture replacement names #258

Merged

Conversation

CosmicHorrorDev
Copy link
Collaborator

@CosmicHorrorDev CosmicHorrorDev commented Oct 26, 2023

Fixes #44

(Still a draft. Needs a lot of cleanup)

This is a mostly working implementation that errors on capture names that are a number followed by a non-number valid capture char (a-zA-Z_). It's a decent chunk of code mostly because regex-automata doesn't expose a replacement capture name iterator (and couldn't without a non-trivial refactoring of their code based on the current implementation). Instead we provide our own which requires a lot of care to match regex-automata's implementation

Anyhoo, here's what the error looks like

image

@CosmicHorrorDev CosmicHorrorDev marked this pull request as ready for review October 28, 2023 20:06
@CosmicHorrorDev
Copy link
Collaborator Author

Alright, I'm quite happy with things now. Most of the code is from implementing our own replacement capture parsing to match upstream or building up the custom error to show the user. Making sure that we match upstream's implementation was the part I was most concerned with, but I'm rather happy with how the tests turned out, so that should hopefully be fine

This should be good enough to merge now 👍

@CosmicHorrorDev CosmicHorrorDev merged commit a88673b into chmln:master Oct 28, 2023
9 checks passed
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.

Bug in replace? Text is removed
1 participant