Skip to content

Python: Support a (ASCII) inline regex flag #15390

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

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

Marcono1234
Copy link
Contributor

Split off from #15345, see description and comments there

Comment on lines 62 to 65
#Group with inline flags
re.compile("(?aimsx:a+)")
re.compile("(?-imsx:a+)")
re.compile("(?a-imsx:a+)")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The - and : in these are not properly handled yet (and therefore some of the .expected entries are incorrect). Should I omit these?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should modify the .expected files to make the tests pass.

you could simply add a comment to this test file (either on line 62 or on each of 63-65) noting that this is not handled correctly.

The machinery to both log your actual expectation and have the test pass does exist in the form of inline expectation tests (where you can write MISSING and SPURIOUS). If you want to go down that route instead, see GroupTest in SubstructureTests.ql and groupTest.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should modify the .expected files to make the tests pass.

I think the tests should pass in their current form, but please let me know if I overlooked something. What I meant was rather that these results are not really 'expected' but just happen to represent the current (slightly incorrect) parsing results.

Thanks for the hint with inline expectation tests; for now I just added a TODO comment in test.py, I hope that is ok.

@yoff yoff self-assigned this Jan 23, 2024
@Marcono1234 Marcono1234 force-pushed the marcono1234/python-ascii-regex-flag branch from 681e9d3 to 1ad08ef Compare January 26, 2024 21:19
Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@yoff yoff merged commit 391ca5d into github:main Jan 29, 2024
@Marcono1234 Marcono1234 deleted the marcono1234/python-ascii-regex-flag branch January 30, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants