fix(unzip): handle sb2 files zipped with a folder #51
Conversation
chrisgarrity
force-pushed
the
handle-sb2-nested-zip
branch
from
e5d9aca to
31a48e4
Compare
|
@kchadha pushed an update that fixes the test that was failing and adds tests. The test was failing because the invalid comments file had been created with MacOS compress, so it had an extra 'hidden' folder. That caused it to fail to load before validating. I created a new Also added a test for parsing an sb2 with a nested folder structure, and one that tests that anything with more than one folder is invalid. Questions:
|
lib/unzip.js
Outdated
| .then(function (zip) { | ||
| return zip.file(isSprite ? 'sprite.json' : 'project.json').async('string') | ||
| const dir = Object.values(zip.files).filter(f => f.dir); | ||
| const projectJson = dir.length === 1 ? dir[0].name + 'project.json' : 'project.json'; |
There was a problem hiding this comment.
Hmmmm... I think I would prefer for the parser to look for the project.json in a dir only if it fails to find a file with the name at the top level. Also, I think we should probably make the same provisions for sprite.json as we do for project.json.
I am not sure if it is possible to find out if zip.file('project.json') actually succeeded or not...
chrisgarrity
force-pushed
the
handle-sb2-nested-zip
branch
from
31a48e4 to
45875e3
Compare
lib/unzip.js
Outdated
| }); | ||
| let file; | ||
| if (isSprite) { | ||
| file = zip.file('sprite.json'); |
There was a problem hiding this comment.
I think we want to treat sprite files the same way we treat project files. I just confirmed that this was something that worked for .sprite2 files in the 2.0 editor.
chrisgarrity
force-pushed
the
handle-sb2-nested-zip
branch
from
45875e3 to
6335962
Compare
Uses a pattern to find the sprite or project json in a zipped folder or flat list of files. Added tests for * loading a sprite or project with a nested folder * loading a file without a json Fixes scratchfoundation#42
chrisgarrity
force-pushed
the
handle-sb2-nested-zip
branch
from
6335962 to
750e866
Compare
|
VM PR is also ready: scratchfoundation/scratch-vm#1993 |
kchadha
left a comment
kchadha
left a comment
There was a problem hiding this comment.
LGTM, thanks for working through the changes. I think this should be pulled into VM via the VM PR (#1993)
|
🎉 This PR is included in version 4.3.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Looks for a directory structure in the zip file, and prepends the name when extracting the project json if necessary. Will not currently work with gzipped files. Project seems to load correctly after validating the json.
Fixes #42
Note: do not use OSX compress as it creates additional ‘hidden’ folders, and loading the file fails.