Skip to content

Conversation

@quaintdev
Copy link

No description provided.

@daenney
Copy link
Contributor

daenney commented Jan 6, 2023

This is really cool. I'm wondering if we can do one better by checking the filesystem first and then falling back to embedded?

For example, it's currently possible to override the templates with whatever you want simply by replacing the file. In my case, I mostly want to edit head.html to include some separate CSS and set some additional meta tags, but I don't need to touch any of the other files.

With the proposed implementation, if I want to override any static file or template, I have to extract and serve everything in c.Dirs.Static or c.Dirs.Template respectively. It would be ideal if I could only have head.html on the filesystem and have everything else served from the binary. That also makes it immediately obvious which template I've actually overridden.

The fs.FS interface is defined as Open(name string) (File, error) so if we can provide a type that implements that by first trying the real filesystem and then the embedded one that might solve it.

Something like:

type fallbackFS []fs.FS

func (ff fallbackFS) Open(name string) (fs.File, error) {

... iterate over the fs'es and try and Open() the file, if it fails move on to the next one. The last FS should be the embed.FS ...
}

And then you'd initialise it like:

staticFS = fallbackFS{os.DirFS(c.Dirs.Static), staticFiles}
...
http.FileServer(staticFS)

@icyphox
Copy link
Owner

icyphox commented Jan 31, 2023

Hey folks, apologies for the absence—moved halfway across the globe so still getting settled down. Still need a few days to sort stuff out and I’ll get to reviewing this. Thanks!

@quaintdev
Copy link
Author

quaintdev commented Mar 22, 2023

@daenney that sort of behavior is ideal but the increase complexity makes me stay away from it. The config option suggested by @gedw99 ensures user has more control than the binary. Also we can have the binary output the template and the config. The user can then just make edits to the template to be overridden in config. We can then just distribute the binary with default config and templates instead of cloning and setting up everything.

@quaintdev
Copy link
Author

For now @icyphox I think we can merge this PR and raise a separate issue for above feature.

@quaintdev
Copy link
Author

quaintdev commented Mar 22, 2023

Upon further testing and analysis I found that this PR is going to break upstream functionality if merged. @icyphox do we need absolute path update of d.c.Templates and d.c.Static paths?

I needed those variables to be empty to check if we should be using embedded templates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants