Conversation
|
PR merged and rebased, ready for review. |
|
Nice, will take a closer look this evening |
|
Pushed an extra tiny commit that renders the full name of directories in the side panel, same as files. Currently, long directory names get truncated and there's no way to see them. |
averne
left a comment
There was a problem hiding this comment.
Thanks a lot, overall this looks very good. I have a few comments, mostly cosmetic change.
The only thing that really needs to be fixed is the handling of the devoptab's internal directory state, but I think the confusion stems from a bug in my existing SMB/NFS code.
I didn't have it in myself to give a proper look at the HTML parsing code, so I'll trust your testing on that.
|
Rebased and pushed some new commits, please check them that way for easier review. Wanted to discuss a choice of behavior with you - currently, SwitchWave will send basic auth preemptively everywhere, without waiting for server challenge. This will make it work with servers which redirect to login portal instead of sending challenge, at the cost of potentially sending credentials even if the server doesn't need them. Worst case, I guess this would send credentials if the user typos the server, but IMO it's quite hypothetical. Do you think this is a reasonable tradeoff? |
Yes, I think that's fine |
|
This should be all comments addressed now. |
|
Great, thanks again for your work |
This PR adds support for HTTP/S shares, the same way as existing SMB, NFS and SFTP. I tested this extensively and saw no issues: nginx/copyparty/Apache autoindex, jpeg/mp4/mkv playback, video, audio, subtitles, skipping back and forth, file metadata parsing via ZL/ZR (ffmpeg).
You will notice that a lot of the FS commands like stat, seek, etc. are not implemented. I tested two approaches here:
I found out that the 2nd approach had a lot faster skipping/scrubbing, buffering, and most importantly, a consistent 10% less CPU usage. I compared with nxmp, which also takes approach 2, and the CPU usage was basically identical.
I did not add size parsing since there's no good way to do it reliably across all autoindex providers.
NOTE: AI was used to help with this PR since I was not familiar with the codebase. I reviewed and tested the code to the best of my abilities, but please definitely check it out as well.
You can test my build here: https://github.com/ViRb3/SwitchWave/actions/runs/22315408241/artifacts/5620623670
Leaving as draft until #11 is merged, as this is based on top of it.