Skip to content
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

Allow `file_server browse` templates to use the same functions as `templates` #3637

Open
francislavoie opened this issue Aug 5, 2020 · 6 comments

Comments

@francislavoie
Copy link
Member

@francislavoie francislavoie commented Aug 5, 2020

See the discussion here: https://caddy.community/t/v2-http-handlers-templates-functions-in-file-server-browse-template/9214

The file_server directive has a browse option which enables a directory index to be served when a directory is requested. By default, this index is rendered using a template which can be found in the code. The default can be overridden with a template provided by the user.

It seems that the file_server browse template doesn't use the same code paths as the templates directive, which means that the functions documented here cannot also be used.

The code should be refactored such that the logic for the templates is shared between these two such that all the template functions Caddy provides works both for the general templates and for the file server directory index.

@divbhasin
Copy link

@divbhasin divbhasin commented Aug 25, 2020

Hi, new contributor here 👋. I can work on this issue.

@francislavoie
Copy link
Member Author

@francislavoie francislavoie commented Aug 25, 2020

Cool! Go for it @divbhasin 😄

@divbhasin
Copy link

@divbhasin divbhasin commented Aug 26, 2020

Here is what I am thinking: I need to make the executeTemplateInBuffer function in templates/tplcontext.go accessible to fileserver browse. This is because that function attaches the other functions like httpInclude to a template, and returns a template that has been executed on a buffer.

Am I on the right track @francislavoie?

@francislavoie
Copy link
Member Author

@francislavoie francislavoie commented Aug 26, 2020

Yeah, that sounds about right. Maybe the templateContext should be shared to allow browse to use it.

@caddyserver caddyserver deleted a comment from vojkuv Aug 30, 2020
@caddyserver caddyserver deleted a comment from vojkuv Aug 30, 2020
@mholt
Copy link
Member

@mholt mholt commented Aug 31, 2020

Hmm, this is tricky though because in order to support of all of templates functions you'd need to import the modules/caddyhttp/templates package and use its Templates type, because templateContext depends on it.

This will need more consideration/discussion; I think there'll need to be some sort of new function or method that is exported to wrap all the unexported logic needed for this, without having to create a whole HTTP handler or something.

@divbhasin
Copy link

@divbhasin divbhasin commented Aug 31, 2020

@mholt yeah I have been struggling with how to incorporate the Templates type into browse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.