fileserver: Browse symlink display shows the target

[armadi1809]

Resolves #5810

I tested this change with both the file_server directive as well as the caddy file-server command and it's working for both. Please let me know if I missed something.

[armadi1809]

Just ran gofumpt -w on the changed file. The linting issue should be resolved.

[mholt]

Thanks for the PR! My only thought is that maybe dereferencing a symlink should be optional. Some people may not want to show the linked path information. How would you feel about implementing a simple config setting for this?

[armadi1809]

@mholt sure thing! One question to make sure I understand this correctly. The config setting will be part of the browse sub-directive syntax? If not, Is there another place in the codebase where we are doing something similar that I can take inspiration from? Thank you for taking the time to review this!

[mholt]

Yeah, it'd likely have to be a subdirective of the browse subdirective. 😅 Maybe something like reveal_symlinks? Or if it's for hard links too, maybe reveal_links?

[armadi1809]

Sounds good. Thanks for the clarification.

[steffenbusch]

Thank you for making this an opt-in. I use the browse template to show certain users/clients what files they can download and some of them are sym-linked to a central location and I would not like to reveal this internal thing.

[mholt]

Thanks for working on this! I'm looking forward to merging this, I just had a few last-minute thoughts before we do.

[mholt]

Actually, how about if we use a Tabler SVG icon for this arrow?

<svg xmlns="http://www.w3.org/2000/svg" class="icon icon-tabler icon-tabler-arrow-narrow-right" width="24" height="24" viewBox="0 0 24 24" stroke-width="2" stroke="currentColor" fill="none" stroke-linecap="round" stroke-linejoin="round"><path stroke="none" d="M0 0h24v24H0z" fill="none"/><path d="M5 12l14 0" /><path d="M15 16l4 -4" /><path d="M15 8l4 4" /></svg>

[armadi1809]

Here, do we just add the svg tag mentioned above to the symLinkRepresentation string or is there another/better way of doing it?

[mholt]

Yeah, just replace the " -> " string with the " <svg ...> " string.

You could move that into a separate variable for readability though.

[armadi1809]

I was just trying this and the " <svg ...> " gets displayed as it is inside the html. I believe the value passed to the go template needs to be of type template.HTML in order for it to parse it as html rather than the raw string. If so, how do you suggest we treat this?

[mholt]

Ah, that's probably true. In that case, the correct way to handle this would be to create a new field in the fileInfo struct, maybe called Symlink or SymlinkName or SymlinkPath or something. Then the UI will render the arrow SVG icon and the symlink info. Does that make sense?

[armadi1809]

@mholt I think I got what you mean. As you can see now, I added a field called SymlinkPath to the fileInfo struct. This new field is a slice of strings that can either be empty or containing two elements (first one being the name of the symlink and the second one being the target of the symlink). This slice gets filled in only when the file is a symlink and the reveal_symlinks directive is enabled. Therefore, in the browse.html file we can check if this slice is empty and in that case display the name otherwise display the first element of the slice followed by theS SVG icon followed by the second element of the slice. Is this a valid approach?

[armadi1809]

@mholt All the feedback items should be addressed now. The only one not resolved yet is the "" vs "->" and as mentioned in the comment above, I was getting the raw "..." string when inserting it directly into the symlink representation as recommended. Thanks again for your continuous help with this!

[mholt]

Thanks! Just had a sit at my desk and was able to comprehend what you meant, I just replied above.

[armadi1809]

@mholt I resolved a conflict inside ./modules/caddyhttp/fileserver/caddyfile.go that emerged after the recent modifications to that file in #5850. This should be ready to be merged again if you still want this feature. Thank you!

[mholt]

This LGTM now. Thanks so much for iterating on this with us, @armadi1809 -- hopefully people will find it useful!