-
-
Notifications
You must be signed in to change notification settings - Fork 290
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
Resolve symlinks on directory listing #479
Conversation
Could you rebase this? |
This has the benefit of showing the size and modification date of the pointed-to file. Symlink to directories now respects '--dirs-first' option and broken symlinks don't show in directory listing.
3df74cc
to
81e6555
Compare
@svenstaro done. |
I quite like this as it is. I like keeping the symlink symbol around. Of course, I'd appreciate a test for this if you want to make one but I'd say for this change it's optional. Once you're happy, remove the |
This should facilitate testing because this symbol will no longer a part of the entry text shown in html.
RFC tag removed! CSS styles may still need some cleanup though: |
Alright, feel free to remove the disused class, no reason to keep it around unless we maybe want to make use of it still? Perhaps we can underline the symlinks in the symlink color? |
Cool stuff with the fixtures! Now perhaps just make one test check whether the symlinks are actually rendered as they should be (while other files and dirs are still rendered as they should be. |
I believe that the current tests would still fail with the old behaviour because symlink filenames do always have a trailing |
Replace some of the testing files and directories with symbolic links. They should behave exactly the same.
efd1acd
to
18a3465
Compare
For non-symlink files and directories, there is no need to call `std::fs::metadata()` as the metadata are already obtained via `entry.metadata()`
I don't really have a strong opinion on symlink symbol style, but it looks ok to me without a special style. |
@aliemjay could you resolve the conflict? I think this was in pretty good shape already the last time I looked at it. |
Looking at this again, I'm thinking: Wouldn't it be kind of cool to render the pointed-to relative location of the symlink behind the arrow? |
I have no strong opinion on this, but a symlink may be absolute or may point outside the serve_path and that would show potentially unwanted path names. |
Perhaps that's something we could show actually in case it's pointing to somewhere that would be off-limits? So we'll end up with these possibilities:
I think it would just be nice to see where a symlink points in case it's valid. :) |
Well, as I said,I have no strong opinion but I see that symlinks should be totally abstracted away from the user and should be used by the admin only to organize the file tree. |
That's a fair point! How about a follow-up PR with a new flag to show symlink destination? :) |
That would be interesting. I shall give it a try! |
This has the benefit of showing the size and modification date of the pointed-to file. Symlink to directories now respects '--dirs-first' option and broken symlinks don't show up in directory listing.
The only downside AFAIK is more syscalls when listing directories.
@svenstaro Any input on this? should it be configurable? can we abandon the symlink symbol?