-
Notifications
You must be signed in to change notification settings - Fork 511
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
Binary File Parsing in read_file_from_workspace
#4977
base: master
Are you sure you want to change the base?
Conversation
@toastx is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
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.
Thanks a lot for contributing!
With this function I think we have now reached the point where it needs to change more fundamentally. What's proposed here returns a String
by default which assumes an encoding, something that can fail in many ways.
Then there is another branch that encodes the content as base64 and returns it in place of the actual content. Now the frontend wouldn't know anymore what to do with it.
To me, it would be ideal if everything is treated as binary and transmitted to the frontend as bytes. There it could decide what to do.
Alternatively, or if this is too cumbersome, encode everything as base64 and handle it in the frontend.
Something that might be better is to return a structure, though, that informs about the assumed kind of data, maybe with a little help of the infer
crate. That could then drive the frontend to indicate what to display.
Following your method, I have a suggestion: So it would output as (content, text), or (base64, binary). Thoughts/Suggestions? |
In my opinion, how about adjusting the format in Rust so that it can be received as a Uint8Array in JavaScript? |
I can try that as well, but I feel using base64/uint8array won't make a difference tbh... And converting the b64 into its original content would be easier no? One more thing is that we can directly convert the binary into a b64 using a one-liner |
You’re right as well. My perspective is that, considering the use of Tauri’s |
can u link me the ipc stuff, ill give it a read for understanding |
It's worth noting that Regarding the return type, I'd prefer returning a struct so there are field-names - they probably bode better in the frontend and make for more readable code. |
https://v2.tauri.app/concept/inter-process-communication/#_top I think this explains the IPC well. |
After reading about the IPC, and the output structure serialization stuff, Serialization of Uint8array would be faster imo, as it is in the raw binary form when it is in Like @Zamoca42 said, handling it in uint8array will be more performance oriented. |
How is a uint8array represented in JSON - the message format of |
Smaller uint8arrays can be wrapped as array, and passed through it. Larger has to either be parsed as b64, or tauri supports the datatype called There is also a method called |
That's wonderful! My takeaway from the issue is that this is still an unsolved problem when using the V1 IPC system. |
All right, Ill begin on the base64 working then. Do let me know if u need anything in particular in the output struct, apart from content, type and name |
So, I want to test the implementation, and I try following the steps from Development.md , but for some reason, I keep on getting error after error, as I keep resolving them. Is there any other guide or way to do it? |
Could you show the logs to see what issues are occurring? |
Did you install the Rust MSVC prerequisites? Assuming that these are installed and working, I admit to have never seen a plain Sometimes it helps to run It's notable that it fails trying to build libz-ng-sys, and with this patch, it shouldn't do that anymore. diff --git a/crates/gitbutler-tauri/Cargo.toml b/crates/gitbutler-tauri/Cargo.toml
index 7f79734e9..0b1328102 100644
--- a/crates/gitbutler-tauri/Cargo.toml
+++ b/crates/gitbutler-tauri/Cargo.toml
@@ -28,7 +28,7 @@ console-subscriber = "0.4.0"
dirs = "5.0.1"
futures.workspace = true
git2.workspace = true
-gix = { workspace = true, features = ["max-performance", "blocking-http-transport-curl", "worktree-mutation"] }
+gix = { workspace = true, features = ["blocking-http-transport-curl", "worktree-mutation"] }
once_cell = "1.19"
reqwest = { version = "0.12.4", features = ["json"] }
serde.workspace = true I hope that helps. |
Yes bro, all the prerequisites have been installed. Have been using cmake for 2-3 projects as well. Not sure why this sudden access is denied |
What about the patch? |
I am still getting the same access denied error, I'll try reinstalling Rust, MSVC and Cmake again tomorrow , and try it |
With the patch applied, |
still no luck man, i uninstalled and reinstalled everything. Idk why its happening tho. If someone else wants to take this over, Im cool with it. |
I was hoping that would be resolved, but it’s unfortunate. Would it be okay if I continue working on it? |
That would be nice, there certainly are no objections from my side. |
☕️ Reasoning
Remove the bail and added a base64 engine to parse binary into a base64 string, which can be displayed in frontend.
🧢 Changes
The function no longer bail at binary, and now returns a b64 string of the image
Fixes: #4957