icyphox / legit

web frontend for git
https://git.icyphox.sh
MIT License
376 stars 25 forks source link

embedded static and template files #18

Open quaintdev opened 1 year ago

daenney commented 1 year ago

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 commented 1 year ago

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!

gedw99 commented 1 year ago

The other way is by using config, then determines if you use embedded or not.

I prefer this rather than complex internal logic. Then we have fully control over what it picks.

quaintdev commented 1 year ago

@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 commented 1 year ago

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

gedw99 commented 1 year ago

Ok :)

quaintdev commented 1 year ago

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.