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

Replace Oniguruma regex patterns with PCRE ones. NOT TESTED!!! #102

Closed
wants to merge 1 commit into from

Conversation

raiph
Copy link

@raiph raiph commented Jul 4, 2023

NOT TESTED!!!

Per #99, Github's regex validator does not accept Oniguruma regexes. There's some escaping/filtering going on that makes the report hard to interpret but I'm pretty sure I figured it out. (I think it's just some \\p{Alpha}s that needed translating to \\pL\\pM, and a "(" and ")" pair of strings that needed escaping of the parens).

NOT TESTED!!!

Per Raku#99, Github's regex validator does not accept Oniguruma regexes. There's
some escaping/filtering going on that makes the report hard to interpret but I'm pretty sure I figured it out. (I think it's just some `\\p{Alpha}`s that needed translating to `\\pL\\pM`, and a `"("` and `")"` pair of strings that needed escaping of the parens).
@2colours
Copy link
Collaborator

2colours commented Jul 6, 2023

I think ultimately we should be moving away from the "atom" part in the name so I'd say nevermind the testing for now.

However, as per CONTRIBUTING.md, the JSON files are only checked in for these external repos to use them; they aren't meant to be altered directly.

@2colours
Copy link
Collaborator

2colours commented Jul 6, 2023

Also, apparently I messed up big time: I uploaded the JSON files with nondeterministic key order. That's hell to revise. The code was already there to treat it with sorted keys...

Anyway, now I did that, after checking all four JSON's that they haven't changed.

@raiph If that's fine with you, I would just modify the corresponding .raku file, while switching the strings to an unquoted version (I didn't do that all at once because it seemed a lot of hassle but it might gradually evolve). This PR can be used as a reference because the same changes should be reflected in the JSON file.

@2colours
Copy link
Collaborator

2colours commented Aug 3, 2023

I did a PR showing what the current "architecture" is and we confirmed it contains the same practical content. Closing this PR now.

@2colours 2colours closed this Aug 3, 2023
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