-
Notifications
You must be signed in to change notification settings - Fork 175
Added Both Revoke and Read function to the Blob Extension #439
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
Changes from 2 commits
1b09c84
b98564f
2483fe3
fbe1f14
f27c4e6
e178b85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,31 @@ | |
| }, | ||
| }, | ||
| }, | ||
| { | ||
| opcode: "readBlob", | ||
| blockType: Scratch.BlockType.REPORTER, | ||
| text: "read blob [URL] as a [TYPE]", | ||
| arguments: { | ||
| URL: { | ||
| type: Scratch.ArgumentType.STRING | ||
| }, | ||
| TYPE: { | ||
| type: Scratch.ArgumentType.STRING, | ||
| defaultValue: "text/plain", | ||
| menu: "types" | ||
| } | ||
| }, | ||
| }, | ||
| { | ||
| opcode: "revokeBlob", | ||
| blockType: Scratch.BlockType.COMMAND, | ||
| text: "Revoke blob with the URL of [URL]", | ||
| arguments: { | ||
| URL: { | ||
| type: Scratch.ArgumentType.STRING | ||
| } | ||
| }, | ||
| }, | ||
| ] | ||
| ,menus: { | ||
| types: { | ||
|
|
@@ -103,10 +128,40 @@ | |
| } | ||
|
|
||
| toBlob(args, util) { | ||
| const text = String(args.DATA); // ensure it's a string (why can't it be just like python or smth) | ||
| const text = String(args.DATA); | ||
| const blob = new Blob([text], { type: args.TYPE }); | ||
| return URL.createObjectURL(blob); | ||
| } | ||
| } | ||
| revokeBlob(args, util){ | ||
| URL.revokeObjectURL(args.URL); | ||
| } | ||
| readBlob(args, util) { | ||
| async function readBlobContent(url, mime) { | ||
| const response = await fetch(url); | ||
| const blob = await response.blob(); | ||
|
|
||
| if (!url.startsWith("blob:")) return ""; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you only just now checking if the |
||
|
|
||
| if (blob.size === 0) return ""; // js for u TheShovel | ||
|
|
||
| if (mime.startsWith("text/") || mime === "application/json") { | ||
| const text = await blob.text(); | ||
| return mime === "application/json" ? text : text; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
|
|
||
| const buffer = await blob.arrayBuffer(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After doing a bit of looking at the MDN documentation for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could also be wrong about this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's worth spending time on concidering the extension already runs really fast and uploading bigger files would be an issue by it's one to be honest, I think it would be better for this chunk of the code to be as is instead of spending time on how we could improve it right now, a think that we can and probably may happen at some point. |
||
| const bytes = new Uint8Array(buffer); | ||
|
|
||
| let binary = ""; | ||
| for (let i = 0; i < bytes.length; i++) { | ||
| binary += String.fromCharCode(bytes[i]); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could just be me, but I think there might be more efficient ways of converting an array buffer into a string. Maybe
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Everything is done! thanks for the feedback btw, I was kind of rushing adding some of the stuff (to which I apologize), from now on I will try to do stuff and put more work into it(which may slow things down because the main reason of which made me rush things up is a really tight time schedule). If you have any other stuff you want me to add I would be more than happy to do so.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That blog article says otherwise? "We found that our new DataView implementation provides almost the same performance as TypedArrays when accessing data aligned in the native endianness (little-endian on Intel processors)" |
||
|
|
||
| return btoa(binary); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the purpose of reading the text as Base64? PenguinMod will be able to handle direct binary-encoded data, pretty sure.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I think that outputting it as b64 could be considered unexpected behavior, mostly because input isn't given in base 64.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so what do you recommend the output being?, if you're just saying to output the raw data then sure I would just return it if you like.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think outputting the raw information is the best option.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The block explicitly casts any input to a string before saving it to an object URL. Whether this is Scratch's or JavaScript's fault is, for all purposes, entirely irrelevant. Non-JS string safe characters cannot be added to an object URL due to the limitations of this extension, and so no non-JS string safe characters couldn't possibly be read from an object URL, unless someone is utilizing another extension that allows for that, or are directly encoding things to an object URL through a JavaScript eval extension. That was my point in the first place.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it was the point of my reply, and what you said there doesnt match what you say now in how you lay it out
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wgat
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. read should not be using string btoa, js strings will naturally corrupt binary data due to how they work
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's what the intended meaning of my point was, if that doesn't come across at all then that's my bad. |
||
| } | ||
|
|
||
| return readBlobContent(args.URL, args.TYPE); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure if this is all you're doing, you don't even need to have another function inside here, just make the function root async; in any case, this will be returning a promise, so it really doesn't matter that you did it this way.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another note about this because I feel like this is important, I'm asking for the method to be made async and for it to directly return rather than routing it through another function. This is all around more readable, at least in my opinion. async readBlob(args, util) {
...
return btoa(binary);
}feel free to disagree, here, though. |
||
| } | ||
| } | ||
| Scratch.extensions.register(new faunks_Blobs()); | ||
| })(Scratch); | ||

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 casing on the reporter here is inconsistent with the other blocks.