Fix tests on windows for using illuminate/filesystem and add option for absolute paths
#176
Conversation
|
It seems like something is still off with reading the files. |
florisbosch
force-pushed
the
main
branch
4 times, most recently
from
fccce3a
to
65be109
Compare
florisbosch
force-pushed
the
main
branch
2 times, most recently
from
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/filesystemand 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.