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

Added GSQL lexer #1809

Merged
merged 3 commits into from Jul 18, 2021
Merged

Added GSQL lexer #1809

merged 3 commits into from Jul 18, 2021

Conversation

DanBarkus
Copy link
Contributor

Added lexer for GSQL language and handling .gsql files

(r'.*\/\*\s*.*\s*\*\/', Comment.Multiline),
],
'keywords': [
(r'(ACCUM|AND|ANY|API|AS|ASC|AVG|BAG|BATCH|BETWEEN|BOOL|BOTH|'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use the words function for long lists of keywords which can optimize the matching, see for instance: https://github.com/pygments/pygments/blob/master/pygments/lexers/python.py#L202


__all__ = ["GSQLLexer"]

class GSQLLexer(RegexLexer):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a comment describing this lexer -- ideally with a link to the language definition. This comment should end with .. versionadded:: 2.10

Choose a reason for hiding this comment

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

@DanBarkus thanks for the great work 😄
@Anteru I can't wait to see this PR Merged 😄

Choose a reason for hiding this comment

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

@DanBarkus Very cool! Can't wait to use it!
@Anteru Looking forward to the merge!

@DanBarkus DanBarkus requested a review from Anteru June 8, 2021 13:20
@DanBarkus
Copy link
Contributor Author

All requested changes have been made.

@JonHerke-TG
Copy link

@DanBarkus thanks for completing this!!
@Anteru It would be great if you could review. We are looking to use this for an O'Reilly Book. Please review as soon as possible. Thanks!

@Anteru Anteru merged commit 710cac7 into pygments:master Jul 18, 2021
Comment on lines +61 to +63
# 'name': [
# (r'(\@\@w+)\b', Name),
# ],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please remove this?

@Anteru
Copy link
Collaborator

Anteru commented Jul 18, 2021

Merged, thanks! I'd appreciate if you could do a follow up commit to trim the example file a bit and reply to the question about name being commented out.

@Anteru Anteru added this to the 2.10 milestone Jul 18, 2021
@Anteru Anteru added changelog-update Items which need to get mentioned in the changelog A-lexing area: changes to individual lexers labels Jul 18, 2021
@Anteru Anteru self-assigned this Jul 18, 2021
@Anteru
Copy link
Collaborator

Anteru commented Jul 18, 2021

I also had to regenerate the golden test files, could you please check that Name.Builtin is acceptable for things like FOR which were Keyword previously?

@Anteru
Copy link
Collaborator

Anteru commented Jul 18, 2021

@DanBarkus This is also failing the docs build as it's missing the top-level comment which includes the copyright annotation. I'm reverting the PR for now until this is addressed. Please reopen once this is resolved.

Anteru added a commit that referenced this pull request Jul 18, 2021
@Anteru
Copy link
Collaborator

Anteru commented Jul 18, 2021

Reverted via 0a93d6c and 066bd7e

@DanBarkus
Copy link
Contributor Author

I've added the missing copyright annotation and removed the unnecessary commented out section of code.
I reviewed uses of the words function in other modules (sql, python) and usage patterns line up with those languages.
Will submit new pull request with updated code.

@Anteru Anteru removed the changelog-update Items which need to get mentioned in the changelog label Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lexing area: changes to individual lexers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants