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 invalid XML regex #417

Merged
merged 1 commit into from
May 23, 2024
Merged

Fix invalid XML regex #417

merged 1 commit into from
May 23, 2024

Conversation

mohd-akram
Copy link
Contributor

@mohd-akram mohd-akram commented Sep 12, 2023

Use the NChar character class to specify Unicode noncharacters rather than specify the ranges manually. This fixes a typo in the first range (should end with \uFDEF not \uFDDF).

See here.

Use the NChar character class to specify Unicode noncharacters rather
than specify the ranges manually. This fixes a typo in the first range
(should end with \uFDEF not \uFDDF).
@mohd-akram mohd-akram changed the title Simplify invalid XML regex Fix invalid XML regex May 22, 2024
@mohd-akram
Copy link
Contributor Author

mohd-akram commented May 22, 2024

I've rebased this onto the latest master. This actually fixes part of the regex so I updated the description as well.

@huntharo
Copy link
Contributor

I added #438 to test this and it shows that, at least, characters that should be removed are removed.

Can we document more about what the NChar character class means / where it came from? I cannot find it defined anywhere. I think we should have a comment that explains how this character class is derived from or related to the docs below as well as the link in the description above that gives the expansion of the character range.

If you want to merge my PR into this one that's fine.

@derduher derduher merged commit dbbdbd9 into ekalinin:master May 23, 2024
3 checks passed
@mohd-akram mohd-akram deleted the simplify-xml-regex branch May 23, 2024 07:32
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.

3 participants