Add a mode that allows rudimentary rendering of images (via unicode text) in bombadillo #131
No reviewers
Labels
No Label
blocked
bug
build
documentation
duplicate
enhancement
finger
gemini
gopher
help wanted
http
in progress
invalid
local
needs-info
non-code
non-functional
non-urgent
question
release
rendering
suggestion
telnet
terminal
urgent
wontfix
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sloum/bombadillo#131
Loading…
Reference in New Issue
No description provided.
Delete Branch "render-images"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
So, this is clearly an experimental feature. I had developed this separately from Bombadillo and then though... hmmm... maybe I can make this work. I don't even know if this is a serious pull request. I'm mostly just making it so that you, @asdf , can try it out and let me know what you think.
Steps to test:
make
./bombadillo gopher://circumlunar.space:70/1/~sloum/test
:set showimages true
(it defaults to false)In case you are wondering, the three images are: FreeBSD Demon, Aeon Flux Eye, and Ruth Bader Ginsburg. Why those three? No idea, I just found them and they looked ok.
The basic rundown of what is happening:
[]string
Notes:
:w .
and the full regular image will be saved (not the braille image). This is intentional, as the braille version is really just to allow people to get an idea of an image inside Bombadillo.Let me know what you think! Again, I know it isnt a highly in demand feature (not a lot of images posted to gopher/gemini), but it was fun to set up. I suppose I should make this work for local files as well... I'll have to think on that.
Even if we do not move forward with images at this time (which I think are a fun optional item for people) I do think some of the code here improves overall performance.
@ -22,2 +24,4 @@
SearchTerm string
SearchIndex int
FileType string
WrapWidth int
Whether or not we move forward with the image rendering I would like this
WrapWidth
int
to be a part of this struct. It lets us keep track of the number of chars the page is wrapped to, which is very useful (as you will see in my next comment).@ -23,1 +25,4 @@
SearchIndex int
FileType string
WrapWidth int
Color bool
This should also be kept for the same reason as
WrapWidth
. It prevents needless render at the cost of a boolean.The previous comment was in regard to the
Color
property. Now that I think about it,FileType
could be changed toImage
and be abool
which might be more lightweight. However, it is possible that keeping the filetype in general might be useful for other things later. I dont think the string should cause too much overhead.@ -66,7 +66,11 @@ func (p *Pages) Render(termHeight, termWidth int, color bool) []string {
}
pos := p.History[p.Position].ScrollPosition
prev := len(p.History[p.Position].WrappedContent)
p.History[p.Position].WrapContent(termWidth, color)
This render method gets called any time the screen draws... so when you scroll, when you open or close bookmarks, etc. Prior to this update, content had to get re-wrapped every single time.
@ -69,1 +69,3 @@
p.History[p.Position].WrapContent(termWidth, color)
if termWidth != p.History[p.Position].WrapWidth || p.History[p.Position].Color != color {
p.History[p.Position].WrapContent(termWidth, color)
Only rewrap if the color mode has changed or the wrap width has changed. This will save a lot of processing and has already made scrolling snappier (particularly for images).
@ -0,0 +277,4 @@
func Render(in []byte, width int) []string {
image.RegisterFormat("jpeg", "jpeg", jpeg.Decode, jpeg.DecodeConfig)
image.RegisterFormat("png", "png", png.Decode, png.DecodeConfig)
image.RegisterFormat("gif", "gif", gif.Decode, gif.DecodeConfig)
I'm not sure if these need to be here. I wonder if go has an
on program execution
kind of constructor-like item available that could run these once on program load rather than each time an image is rendered.I think this is pretty useful - it's a convenient way to view images, especially in a terminal-only setting.
Your comment regarding the demand of such a feature, maybe people aren't posting many images because of the terminal only expectation. It's possible that a feature like this could actually increase demand.
It seems like it could work well enabled by default - if the user follows the link, the image is rendered. If they just want to download the image, they can
:w
the link number.Also, the handling of the images - downloading using
:w .
gives you the actual image file, is really good.What made you consider putting this in as part of bombadillo, versus having it external and feeding the rendered content back in (similar to how lynx works)?
Awesome! I'm glad you like it. I also have found it really convenient to get the basic idea of an image without having to save a file.
I am definitely open to enabling by default. That way more people will stumble upon it and can always turn it off if they like.
Since I had developed it separately I could see that it wasn't really that much code (it is actually a bit less than the standalone version), so adding it in to the program creates a lower barrier to entry for a feature that I wanted. Since it is not a common program like lynx or w3m or telnet I felt like it just wouldn't be a feature that would get used unless I provided the code as a part of the package.
Do you think it would be a better idea to have it be an outside program and just encourage people to install it as an add-on if they want it?
Tildegit had trouble switching what branch this was merging into. I want to create a release branch with this and a few other things so I am closing this and migrating to a new pr that goes into
release214
Pull request closed