Closed samnorr closed 7 months ago
samnorr
commented
8 months ago samnorr
commented
7 months ago How have you tested your changes? As fas as i can see, you have changed atleast the full_startup_path for everything. So have you tested it and nothing broke?
Otherwise, nice PR and thanks for you contribution. I´ll merge this as soon as you answered my questions :D
I have done a bunch of testing with ZIP files (which is what my changes were focused on), but also a few tests with EXE and PDF, just to make sure the other types still worked as before.
The Full_Startup_Path change was really just a simple renaming of the variable to Full_Startup_Path_Quoted. Which was so that the un-quoted version could then be used for the ZIP command - for the other file types, the logic is still identical to before. Again, the quoting tweaks to the command format would likely make sense to apply to the other file type cases as well, but I just wanted to limit the scope of my changes for now.
Joly0
commented
7 months ago I have done a bunch of testing with ZIP files (which is what my changes were focused on), but also a few tests with EXE and PDF, just to make sure the other types still worked as before.
Ok, that sounds good, i´ll test that aswell, and see if everything still works
The Full_Startup_Path change was really just a simple renaming of the variable to Full_Startup_Path_Quoted. Which was so that the un-quoted version could then be used for the ZIP command - for the other file types, the logic is still identical to before. Again, the quoting tweaks to the command format would likely make sense to apply to the other file type cases as well, but I just wanted to limit the scope of my changes for now.
Ok, i dont think it will make a difference for most use cases, but i am not sure, if intunewin files will work so easily. I have to test this, before i can approve your pr, but i that needs to wait till next week.
Just another thing: I am talking with damien van robaeys, the original creator of Run in Sandbox. The upstream repo doesnt have a licence, which usually makes it kinda hard to do anything with this source code and technically, he has the copyright basically. I am talking with him, that he adds a licence to his project and i would adopt that in this fork. Now i would need your clear consent, that you are fine with whatever licence we would decide to use in the project. Otherwise i would have to wait, to add your code to this project
samnorr
commented
7 months ago Just another thing: I am talking with damien van robaeys, the original creator of Run in Sandbox. The upstream repo doesnt have a licence, which usually makes it kinda hard to do anything with this source code and technically, he has the copyright basically. I am talking with him, that he adds a licence to his project and i would adopt that in this fork. Now i would need your clear consent, that you are fine with whatever licence we would decide to use in the project. Otherwise i would have to wait, to add your code to this project
No concerns from me - I am fine with whatever licence is decided.
Thanks for your efforts in helping keep this project alive! I only started using Windows Sandbox fairly recently, as I hadn't realised just how much of a useful feature it is, especially for trying out applications... And RiS makes it all the more quick and easy to use (the context menu options really should have been included in the native implementation). 😊
Joly0
commented
7 months ago Btw, i am just going through all my usual test files and so far everything looks good, though i found a bug with .sdbapp files. Not a bug which you caused, but which was caused by upstream reverting all my changes to the code for that file type. So i have to get this working again first, then i can merge your pr if everything works as expected
Joly0
commented
7 months ago Ok, everything seems to be working