-
-
Notifications
You must be signed in to change notification settings - Fork 255
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
Fix tests on windows for using illuminate/filesystem
and add option for absolute paths
#176
base: main
Are you sure you want to change the base?
Conversation
It seems like something is still off with reading the files. |
fccce3a
to
65be109
Compare
bc482f5
to
4a74b52
Compare
Hi @arukompas, I can't really debug this problem because I cannot run the windows containers on my Macbook at the moment. Sorry for the long wait on this issue! |
Hey @florisbosch , thank you so much for the effort and please don't fret it and enjoy your holiday :) I'm off as well, and if I need to tag another release/fix, I can just revert the previous merge and we can re-apply it later. Enjoy your holidays! 🎉 |
Hi @arukompas, I managed to setup a VM with windows and fixed a few test. Do you have any idea what can cause this problem? |
Hi @arukompas, |
Related: #174
This pull request fixes tests on windows for using
illuminate/filesystem
and adds an option for absolute paths.The fix for windows was to check for existing files before trying to use the filesize or lastmodified functions.
I've added the option to enable/disable absolute paths. It is working on my machine but I don't think this is the most optimal or elegant way to solve this problem. With this solution the current features are backwards compatible without any changes in current implementations.
My idea for handling this problem in the future is to allow for the combination of multiple disks, this way we can use multiple local or cloud folders in the same viewer. Using absolute paths is usually not the most secure way of handling files especially when you are not fully in control of the hosting of your Laravel application.