-
Notifications
You must be signed in to change notification settings - Fork 5
add multi-upload support #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Just noticed that not all files are added to the db. All items read in the db and then write it again, overwriting each other |
Emeraude
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks interesting.
You should update it to make it compatible with the Emeraude/master branch.
| updateMetadata: (e) => | ||
| newdiv = '<div id="tagform">' | ||
| for i in e.files | ||
| console.log(i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This king of debug should be removed.
| metadata.artist = document.getElementById('artist').value | ||
| metadata.title = document.getElementById('title').value | ||
| metadata.artist = form[0].value | ||
| metadata.title = form[1].value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Track number should not be dismissed (it will be used later).
|
Did you manage to figure out why some the db is not correctly updated with your version ? |
|
I think I got it. It's because calls to ZeroNet API are asynchronous, and variable |
|
That is the issue i first ran into, but line 135 uses the "do" which makes the variables "metadata, path, i" static to the inner part (not sure if clear enough). I think the problem is: In line 140 it reads the file contents.json, if all iteration read the file, change it and write it back. they only write their own modifications |
|
Yes, that's probably true. |
|
Maybe its a good idea to only start uploading the next one when the first one is done. and keep the contents.json in mem while uploading. |
|
We can upload all the files at the same time, but we have to update the |
|
Maybe: and at the end of the callback: |
|
I'd be happy to merge this PR if you correct the bug with that piece of code above, and if you correct the conflicts with the Emeraude/master branch (it's only about the |
|
Will do that, after my exams. |
|
Are you still working on it? |
Don't know if you already want to implement this, or if this is the best way. But here is my version for multi-upload.
I saw you just updated the repo making mine out of sync. If you want to merge this ill update it. and if you dont, well i had fun making this :).
to test: http://localhost:43110/152qbnvTwEDHoDSecNQhYoR6z78QVLpE4f