Closed triforce closed 1 year ago
@nemasu please test this thoroughly if you have a chance.
This looks great! I will start testing it shortly.
Sorry for the delay, I got around to testing it a bit.
I ran into a problem when I did something like this:
cd web_root; mkdir dir1; touch dir1/{a,b,c}
If I go to localhost/dir1
the listing works, but when I click on one of them,
it tries to go to localhost/dir1/a/
and then 404s instead of downloading a.
It think's 'a' is a directory for some reason. I will look into it.
Could you retest please @nemasu? It turned out to be quite a simple fix in the end!
Hello, sorry for the super late reply. I did some retesting, seems to be working so far!
There's 2 potential things though: -Putting the previous directory ("...") at the top of the list. -If 'index.html' exists in a directory, display that as default.
What do you think?
Yes sounds fine to me. So presumably when in the web_root there would be no previous directory link shown? Other web servers have directory listing as a config option as well so perhaps we should start planning a way to turn features on and off after this?
Yeah that makes sense. Funny you should mention config options, I was just thinking about how to go about disabling features to reduce binary size.
To make thing easier though, I think we should do optional features as a separate issue after merging the directory code.
Yep makes sense.
@nemasu feel free to make those changes, I haven't had time recently unfortunately.
Okay, I'll see if I can.
This is teetering on the edge of being finished @nemasu. There is just these two issues which you raised outstanding:
-Putting the previous directory ("...") at the top of the list. -If 'index.html' exists in a directory, display that as default.
If anyone has some time then feel free to jump in and implement those two issues.
@nemasu after studying the code more closely it looks like requests to the root directory (and nothing else) adds the default document (index.html), so as part of the outstanding changes still required here we should remove that logic and integrate it with the directory listing functionality.
Hello @triforce, I'm finally looking at getting this committed, there are merge conflicts however since it's been ... a while, to say the least (sorry about that).
I've made your changes into a patch file relative to the current HEAD, if you like, you can close this PR and make a new one with the updated patch, and I'll accept it and work on the additions after. Of course, if you want, you can use this PR too, I just don't really know an easy way to do that.
Merged in PR #58 .