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

A fix for black formatter breaking standalone parser generation. #1363

Closed

Conversation

marsninja
Copy link

Here lies a fix for Lark's standalone parser generation breaking when Lark is run through the black formatter. The fix simply entails stripping lines as they are iterated through in the extract_sections function in standlone.py.

@marsninja
Copy link
Author

marsninja commented Nov 12, 2023

Closes #1362

@MegaIng
Copy link
Member

MegaIng commented Nov 12, 2023

Or... You don't run black on this codebase, which in this case destroys information. I presume the problem is that black indents the comments, which is just wrong behavior since the comments do act on a global scope.

@marsninja
Copy link
Author

@MegaIng Thats correct, the comments get tabbed in with black, however the strip fix appears to correct the issue for both Lark proper and standalone. Doesn't hurt to make the implementation more black friendly.

@erezsh
Copy link
Member

erezsh commented Nov 13, 2023

@marsninja Why not just run black after the standalone has been generated?

@MegaIng
Copy link
Member

MegaIng commented Nov 13, 2023

@MegaIng Thats correct, the comments get tabbed in with black, however the strip fix appears to correct the issue for both Lark proper and standalone. Doesn't hurt to make the implementation more black friendly.

Sure, it technically doesn't hurt. But it is extra code, and it allows something that currently isn't allowed, with good reasons. Having those comments be indented is confusing for human parsers, so it shouldn't be allowed. Considering the alternative solution to this "issue" is "just don't do the thing that breaks it, there is no reason to do it", I don't see why we should add this.

@erezsh
Copy link
Member

erezsh commented Mar 29, 2024

@marsninja Please address the concerns of the maintainers, or I will close this PR. Thanks.

@m42e
Copy link

m42e commented Jun 20, 2024

Here is something

@erezsh erezsh closed this Jun 20, 2024
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.

4 participants