Refactor scripts in Tools/peg_generator/scripts#20401
Conversation
|
Is it okay to not open an issue for this? |
Why not use the same bpo issue we've been using for almost everything? Then again, labeling as "skip issue" sounds fine too. |
That's a valid question. I've been trying to refrain from doing so generally. I'm thinking about closing that issue as soon as #20106 is merged.
I'm going to do that then. |
|
Btw, I noticed that the |
|
Also, we could remove the need to parse the grammar to |
Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
… dot syntax error
Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
|
Friendly ping. |
gvanrossum
left a comment
There was a problem hiding this comment.
Sorry for taking so long! This is a great improvement to the readability of this code. I have a few comments.
| if new_parser: | ||
| tree = _peg_parser.parse_string(program) | ||
|
|
||
| if args.diff: |
There was a problem hiding this comment.
I'm actually curious if we shouldn't swap a and b here. Wouldn't it make more sense if a was the old parser's tree and b was the new parser's tree? That's how we usually view diffs -- a (or "left") is the old code, b ("right") is the new code.
| print("# Trees are the same") | ||
| else: | ||
| print(f"# Parsed using {args.grammar_file}") | ||
| print(f"# Parsed using the new parser") |
There was a problem hiding this comment.
Doesn't need to be an f-string any more.
| return 1 | ||
| def parse_file(source: str, file: str, mode: int, oldparser: bool) -> Tuple[Any, float]: | ||
| t0 = time.time() | ||
| if mode == 2: |
There was a problem hiding this comment.
I wonder if we shouldn't introduce an enum or at least named constants for the mode values. I am already forgetting what 2 means here.
| else: | ||
| print( | ||
| "A grammar file or a tokens file was not provided - attempting to use existing parser from stdlib...\n" | ||
| result = _peg_parser.parse_string( |
There was a problem hiding this comment.
In the pegenvm branch I introduced a new flag to parse_string, ast, which if set to False doesn't produce a parse tree. This would be useful for mode=0. It also makes test parses of xxl.py much quicker (converting the tree to Python objects takes more time than the actual parsing).
|
I added support for |
|
Thanks @lysnikolaou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
|
GH-20671 is a backport of this pull request to the 3.9 branch. |
(cherry picked from commit ba6fd87) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Automerge-Triggered-By: @gvanrossum