bpo-36143: Regenerate Lib/keyword.py from the Grammar and Tokens file using pgen#12456
bpo-36143: Regenerate Lib/keyword.py from the Grammar and Tokens file using pgen#12456pablogsal merged 5 commits intopython:masterfrom
Conversation
141e73f to
b619b3f
Compare
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
Travis CI tests passed on this PR: it runs "make regean-all" and make sure that generated files are unchanged. So it means that your generator works ;-)
I proposed minor coding style changes, it's up to you to ignore them or not :-)
vstinner
left a comment
There was a problem hiding this comment.
LGTM. It's now perfect, thanks :-)
| ) | ||
| parser.add_argument( | ||
| "keyword_file", | ||
| type=argparse.FileType('w'), |
There was a problem hiding this comment.
Would be nice to make the script working on read-only sources (doing nothing if the file is not changed). See for example Tools/scripts/generate_token.py.
There was a problem hiding this comment.
Sorry, I am not sure I understand exactly what do you mean :(
Can you elaborate a bit more? Or if you want to do it directly, please, push directly to my branch to update the PR :)
There was a problem hiding this comment.
See "make regen-opcode": Antoine Pitrou wrote $(UPDATE_FILE) which leaves the file unchanged if the content didn't change. One advantage is to not touch the modification time of the file at all. It matters for Makefile to avoid useless recompilation.
There was a problem hiding this comment.
I propose to use function update_file() from Tools/scripts/generate_token.py.
There was a problem hiding this comment.
Although I like update_file() approach, I also like the fact that $(UPDATE_FILE) will check that the files are updated in every PR if someone changes the generator as this has been very valuable for gramminit.c and friends.
| 'while', | ||
| 'with', | ||
| 'yield' | ||
| ] |
There was a problem hiding this comment.
@serhiy-storchaka: Sorry, I'm the one who asked @pablogsal to write 'yield']. It doesn't matter, I'm fine with "]" on a separated line as well.
| ) | ||
| parser.add_argument( | ||
| "keyword_file", | ||
| type=argparse.FileType('w'), |
There was a problem hiding this comment.
See "make regen-opcode": Antoine Pitrou wrote $(UPDATE_FILE) which leaves the file unchanged if the content didn't change. One advantage is to not touch the modification time of the file at all. It matters for Makefile to avoid useless recompilation.
https://bugs.python.org/issue36143