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

Refine the formatting of generated Python3 code #4708

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

pelson
Copy link
Contributor

@pelson pelson commented Oct 1, 2024

This handles mostly whitespace improvement to be more aligned with PEP-8 by default. It doesn't yet fix a number of the newline issues that crop up.

Additionally, it improves the typing of a number of obvious methods. The typing remains incomplete however.

The changes to XPathLexer.py are fully auto-generated (with the exception of the checkVersion call).

In the future, I would work towards removing the from antlr4 import * and replacing it with named imports.

I have validated this against a mildly complex parser grammar in https://github.com/SciTools/cf-units/blob/main/cf_units/_udunits2_parser/udunits2Parser.g4.

@pelson pelson changed the title Refine the generated Python3 code Refine the formatting of generated Python3 code Oct 1, 2024
@@ -829,7 +874,7 @@ class <lexer.name>(<if(superClass)><superClass><else>Lexer<endif>):
SerializedATN(model) ::= <<
def serializedATN():
return [
<model.serialized: {s | <s>}; separator=",", wrap>
<model.serialized: {s | <s>}; separator=", ", wrap>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found this rule to be close to what I wanted, except that there remains a trailing whitespace when a line is wrapped. I didn't know how to fix this, and figured it was good enough for now.

@ericvergnaud
Copy link
Contributor

Thanks for this but it breaks the build

@ericvergnaud
Copy link
Contributor

Hi, looks like you need to rebase this PR ?

@pelson
Copy link
Contributor Author

pelson commented Nov 19, 2024

Hi, looks like you need to rebase this PR ?

Yes, it was related to #4726 which touched some code that got removed in this PR. Should be all fixed.

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.

2 participants