search

sissaschool / elementpath

XPath 1.0/2.0/3.0/3.1 parsers and selectors for ElementTree and lxml
MIT License
72 stars 20 forks source link

Issue translating regular expression (via XMLSchema) #75

Closed RS-Credentive closed 2 months ago

RS-Credentive commented 3 months ago

https://github.com/sissaschool/elementpath/blob/4a166804290924b07067433e4d2b88723bb3567f/elementpath/regex/patterns.py#L125

Hi, I'm running into a problem in xmlschema, but I think it may be related to this library.

I am developing an application to generate code from a specific XML schema, and I need to extract patterns from an XSD into python regular expressions for injection into templates.

Generally, it works quite well! However, I have noticed an issue with the pattern \\S(.*\\S)?, intended to describe a string with no leading or trailing spaces. This pattern is translated into ^(?:\\S(?:[^\r\n]*\\S)?)$(?!\\n\\Z) - note that the [^\r\n] in the middle of the string does not have a double escape.

I believe the line I reference above is the culprit - it does not use a raw string, like the rest of the examples in this module. The result is that this sequence is translated into:

"^(?:\S(?:[^
]*\S)?)$(?!\n\Z)")

when I pass it to another library. Does this appear to be the issue? I can submit a pull request, but wanted to see if you agree with my troubleshooting.

Thanks!

RS-Credentive commented 3 months ago

I have confirmed this fixes the issue. Submitting PR

brunato commented 3 months ago

Hi, thank you for reporting the issue and for the PR. If you can fix the failing tests with another commit in the PR (then i squash and merge the PR into a single commit anyway) I would appreciate.

RS-Credentive commented 3 months ago

I have added what I believe will fix the failing tests, but I'm not sure how to trigger a re-test.

brunato commented 3 months ago

You cannot trigger CI tests on the origin repo, maybe you could set something in your fork project. But for this case running local tests in the usual mode for unittest or with tox is sufficient (the fix is not strictly related to the platform or the version of Python).

I'll keep this issue open until the next release of the package is published.

thank you

RS-Credentive commented 3 months ago

It appears that the tests have run successfully for the updated PR! Let me know if you have any other concerns.

brunato commented 2 months ago

Hi, a new release of the package is available, so i close the issue.