Skip to content

Lex annotation namespaces #3

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

Conversation

ShortDevelopment
Copy link
Contributor

According to the OpenQASM 3 grammar the annotation keyword does support namespaces by chaining Identifiers using the . symbol.
https://github.com/openqasm/openqasm/blob/12de90c301500bd01b9f65bdcbbf4a5ec5c2fec9/source/grammar/qasm3Lexer.g4#L42

But the regex in the pygments lexer does not allow this.

(r"^[ \t]*@\w+", token.Name.Decorator, "annotation"),


Fixes #2

@CLAassistant
Copy link

CLAassistant commented May 13, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Cool, thanks! Are you ok adding a release note? If you pip install reno, then it's reno new --edit lex-namespace-annotations and you can write a sentence under "features" (and delete the rest). No worries if not - I can do it.

@ShortDevelopment
Copy link
Contributor Author

Sure, I added a new entry under "fixes" but I can change that if you want.

Currently ci fails because it cannot find python 3.7?

@jakelishman
Copy link
Contributor

oh boy wow, I have not taken care of this repo haha. I guess let me bring the CI up to present day and then I'll merge this - thanks for the changes!

@jakelishman
Copy link
Contributor

You've got the "allow pushes from maintainers" unchecked (I think?) so I can't push a merge commit to your branch, but if you either rebase over or merge up main, I think this should pass and I can merge it now.

@ShortDevelopment
Copy link
Contributor Author

Sry I actually don't even have this checkmark...
(Maybe that's cause I forked into an org I don't control)

Copy link
Contributor

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

yeah, that'd be it - you have to have enough permissions to grant people write access to your repo.

Thanks for the quick changes - I'll merge this, make sure the switch, case and default keywords lex (pretty sure those were added since the last time I updated this package), and then cut a new release.

@jakelishman jakelishman merged commit b203921 into openqasm:main May 14, 2025
4 checks passed
@ShortDevelopment ShortDevelopment deleted the bugfix/annotation-namespaces/2 branch May 14, 2025 16:59
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.

OpenQASM 3 annotations should allow namespaces
3 participants