imgcat
imgcat copied to clipboard
trashhalo
Added support for custom image loaders
Took a bit longer than I expected. I ended up going for a whole interface around it, which you can implement yourself in any way you'd like.
@muesli since you are the first 3rd party consumer of this API could you review this PR to confirm it meets what your use case for glow?
I think I would have just gone for an io.Reader. That's all we need to load data. The wrappers for http / file loaders around it can be tiny then and we remove a lot of duplicated code.
Now that I look at it again I should probably have made the HttpImage accept a custom http.Client anyway. I mostly just figured that the file and http loaders were gonna be needed to keep the command line tool working anyway. So wouldn't make a huge difference whether it would be exposed in the library or just in the command line tool imo.
Understood. I'll quickly whip up an example with a simple io.Reader approach for comparison.
Just as a heads up: I skipped the other improvements @schoentoon introduced. Nice work there, I'm all for them!
@schoentoon You got some gnarly conflicts at the moment. For this to get merged. Changes I'd like to see:
- Obvi conflicts addressed
- Lib code moved back to root OR follow go standard layout
I have some bandwidth this weekend to push this forward if you are swamped.
TIL /lib is non-standard. I'll move it to /pkg later today and address the conflicts.
@schoentoon Yeah, it's all conventions, but I guess it makes sense to stick to the established ones. Sorry for causing the merge conflicts here, but I hope you can see why using the existing io.ReadCloser interface is sufficient in this case: it enables compatibility with an entire ecosystem of existing Go packages that all support this very interface (or the easily NilCloser wrapped io.Reader).
For this reason I think we should try not to introduce an ImageLoader interface here unless we got a really good reason for it.
I would argue that being able to provide a image.Image directly (or a gif.GIF for that matter) would be nice to have as well. Take for example wanting to apply certain filters over an image somewhere within your program. You'll probably already have a image.Image in such a case. Not having to encode it to later on decode it again seems like a benefit to me.