tools/file_packager.py: Cleanup code and and its generated output. NFC #16034
Conversation
sbc100
commented
Jan 15, 2022
sbc100
commented
- Use enumerate() to avoid a counter
- (mostly) Fix indentation of generated JS
sbc100
force-pushed
the
cleanup_file_packager
branch
from
ab86c80 to
d539534
Compare
|
View with "hide whitespace" since this diff has a bunch of white-space only changes to fix indentation. |
sbc100
force-pushed
the
cleanup_file_packager
branch
from
d539534 to
6c2ae7e
Compare
sbc100
force-pushed
the
cleanup_file_packager
branch
from
6c2ae7e to
9017163
Compare
sbc100
changed the title
| @@ -282,8 +282,7 @@ def main(): | |||
| # standalone calls | |||
| if not from_emcc: | |||
| ret = ''' | |||
| var Module = typeof %(EXPORT_NAME)s !== 'undefined' ? %(EXPORT_NAME)s : {}; | |||
| ''' % {"EXPORT_NAME": export_name} | |||
| var Module = typeof %(EXPORT_NAME)s !== 'undefined' ? %(EXPORT_NAME)s : {};\n''' % {"EXPORT_NAME": export_name} | |||
There was a problem hiding this comment.
I'm confused about this line change. Before the change we have real newlines, while after this we still have one real newline (after ''') and one escaped one at the end of the string. What is the benefit to the refactor?
There was a problem hiding this comment.
The problem with the old code is the three extra chars of whitespace at the end of the string. i.e. The new string ends in a newline.. the old string end in three space. The three spaces IIUC were there to avoid the """ being in column zero or the python code.
This change removes all of this pattern of "newline + some extra white space" in favor just a newline, and at the same time this reduces the overall number of lines in the file.
If you look at the resulting output you will see it is more readable with less random trailing whitespace.
An alternative to this approach would be make ret inton and array and use += in it and then use '\n'.join(ret) at the end of the file. This would avoid all the terminating newlines. Maybe I should give that a go?
There was a problem hiding this comment.
The three spaces IIUC were there to avoid the """ being in column zero or the python code.
Can we just remove the three spaces then? Python accepts that IIANM.
There was a problem hiding this comment.
Yes, but the problem is that we end up with lots of stuff in column 1 of the python code.. which IMHO makes it harder to read in general.. and add a lot of extra lines to the lines don't do anything.
I've updated the this PR to be even more consistent and added quick test for trailing/extra whitespace in the output.
There was a problem hiding this comment.
sgtm, I'm mostly neutral here, so I don't want to block this. Good idea about the test.
sbc100
force-pushed
the
cleanup_file_packager
branch
from
9017163 to
f4a5408
Compare
- Use enumerate() to avoid a counter - (mostly) Fix indentation of generated JS
sbc100
force-pushed
the
cleanup_file_packager
branch
from
f4a5408 to
2923d7e
Compare
| @@ -282,8 +282,7 @@ def main(): | |||
| # standalone calls | |||
| if not from_emcc: | |||
| ret = ''' | |||
| var Module = typeof %(EXPORT_NAME)s !== 'undefined' ? %(EXPORT_NAME)s : {}; | |||
| ''' % {"EXPORT_NAME": export_name} | |||
| var Module = typeof %(EXPORT_NAME)s !== 'undefined' ? %(EXPORT_NAME)s : {};\n''' % {"EXPORT_NAME": export_name} | |||
There was a problem hiding this comment.
sgtm, I'm mostly neutral here, so I don't want to block this. Good idea about the test.
|
This is mostly just cleanup to make things more readable for me.. in preparation for actual file_packager changes to follow. |
