Skip to content
This repository was archived by the owner on Oct 8, 2024. It is now read-only.

Add script to fix animations on windows #383

Open
WillB97 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add script to fix animations on windows #383

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a101e33
Add script to fix animations on windows
WillB97 Feb 18, 2023
55d2984
Linting
WillB97 Feb 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions script/fix-animation
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#!/usr/bin/env python3
import re
import argparse


def fix_animation(input: str, output: str) -> None: # noqa: A002
with open(input) as f:
data = f.readlines()

output_data = []
for line in data:

@PeterJCLaw PeterJCLaw Feb 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

General style point for learning: I would encourage thinking about separating the logic which processes the content of the files from the logic which handles the file I/O. Doing so tends to make the code clearer as well as making the processing logic more easily reusable.
For this case I'd probably go with a separate function which operates on a single line at a time (and have a comment explicitly noting the assumption that the HTML elements of interest are each on a single line), keeping the loop in the same function as the IO.

if 'stylesheet' in line:
match = re.search(r'href="(.*?)"', line)
if match is None:
output_data.append(line)
else:
css_name = match[1].replace('\\', '/').split('/')[-1]
output_data.append(f' <link rel="stylesheet" href="{css_name}">\n')
Comment on lines +17 to +18

@PeterJCLaw PeterJCLaw Feb 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Feels like it would be safer to do a find & replace on the adjusted text rather than fully rebuilding the line. That would avoid any issues with the underlying template changing (also below).

elif '<webots-view' in line:
match = re.search(
r'data-thumbnail=(.*?) .*data-scene=(.*?) .*data-animation=(.*?)>', line)

@PeterJCLaw PeterJCLaw Feb 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: please could we wrap this for consistency:

Suggested change
r'data-thumbnail=(.*?) .*data-scene=(.*?) .*data-animation=(.*?)>', line)
r'data-thumbnail=(.*?) .*data-scene=(.*?) .*data-animation=(.*?)>',
line,
)

if match is None:
output_data.append(line)
else:
jpg_name = match[1].replace('\\', '/').split('/')[-1]
x3d_name = match[2].replace('\\', '/').split('/')[-1]
json_name = match[3].replace('\\', '/').split('/')[-1]
Comment on lines +25 to +27

@PeterJCLaw PeterJCLaw Feb 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe worth asserting that the removed portion of these is all the same?
Perhaps like:

Suggested change
jpg_name = match[1].replace('\\', '/').split('/')[-1]
x3d_name = match[2].replace('\\', '/').split('/')[-1]
json_name = match[3].replace('\\', '/').split('/')[-1]
*jpg_dir, jpg_name = match[1].replace('\\', '/').split('/')
*x3d_dir, x3d_name = match[2].replace('\\', '/').split('/')
*json_dir, json_name = match[3].replace('\\', '/').split('/')
assert jpg_dir == x3d_dir == json_dir

output_data.append(
f' <webots-view data-thumbnail={jpg_name} '
f'data-scene={x3d_name} data-animation={json_name}></webots-view>\n',
)
else:
output_data.append(line)

with open(output, 'w') as f:
f.writelines(output_data)


def main() -> None:
parser = argparse.ArgumentParser()

parser.add_argument('animation')
parser.add_argument('output')
Comment on lines +42 to +43

@PeterJCLaw PeterJCLaw Feb 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For learnings: you might be interested in argparse.FileType, a class which can be passed to the type kwarg here. It handles opening the files as well as natively supporting using - for STDIN and or STDOUT.


args = parser.parse_args()

fix_animation(args.animation, args.output)


if __name__ == '__main__':
main()