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

Request: support hologram.nvim as image provider #14

Closed
tzachar opened this issue Aug 12, 2021 · 11 comments
Closed

Request: support hologram.nvim as image provider #14

tzachar opened this issue Aug 12, 2021 · 11 comments
Labels
enhancement New feature or request output-chunks This is related to how we display some mimetype

Comments

@tzachar
Copy link
Contributor

tzachar commented Aug 12, 2021

Use c31b19e to implement support

@tzachar
Copy link
Contributor Author

tzachar commented Aug 12, 2021

this adds initial support for hologram. However, hologram does not seem to be mature enough :(
diff --git a/rplugin/python3/magma/init.py b/rplugin/python3/magma/init.py

index 9a7bb08..fa613ee 100644
--- a/rplugin/python3/magma/__init__.py
+++ b/rplugin/python3/magma/__init__.py
@@ -41,7 +41,7 @@ class Magma:
 
         self.options = MagmaOptions(self.nvim)
 
-        self.canvas = get_canvas_given_provider(self.options.image_provider)
+        self.canvas = get_canvas_given_provider(self.options.image_provider, self.nvim)
         self.canvas.__enter__()
 
         self.highlight_namespace = self.nvim.funcs.nvim_create_namespace("magma-highlights")
diff --git a/rplugin/python3/magma/images.py b/rplugin/python3/magma/images.py
index 0944479..8ad87ba 100644
--- a/rplugin/python3/magma/images.py
+++ b/rplugin/python3/magma/images.py
@@ -1,6 +1,8 @@
-from typing import Set, Dict
+from typing import Set, Dict, List, Any
 import os
 from abc import ABC, abstractmethod
+from pynvim import Nvim
+import dataclasses
 
 from magma.utils import MagmaException
 
@@ -93,8 +95,66 @@ class UeberzugCanvas(Canvas):
             self._to_make_visible.add(identifier)
 
 
-def get_canvas_given_provider(name: str) -> Canvas:
+@dataclasses.dataclass
+class HologramImage:
+    id: int
+    width: int
+    height: int
+
+
+class Hologram(Canvas):
+    nvim: Nvim
+    images: List[HologramImage]
+
+    def __init__(self, nvim):
+        self.nvim = nvim
+        self.images = []
+        # this is better placed in a seperate lua file
+        nvim.exec_lua("""
+            local images = {}
+            local function new(path, x, y)
+                local image = require('hologram.image'):new({source=path, row=10, col=0})
+                --local image = require('hologram.image'):new({source=path, row=x, col=y})
+                local id = #images
+                images[id] = image
+                return id
+            end
+
+            local function show(image_id, width, height)
+                images[image_id]:transmit({width=width, height=height})
+            end
+
+            local function clear()
+                for _, image in pairs(images) do
+                      image:delete(image.id)
+                end
+            end
+
+            hologram = { new=new, show=show, clear=clear }
+        """) # type: ignore
+
+    def __enter__(self, *args):
+        return self
+
+    def __exit__(self, *args):
+        return
+
+    def present(self) -> None:
+        for image in self.images:
+            self.nvim.lua.hologram.show(image.id, image.width, image.height)
+
+    def clear(self):
+        self.nvim.lua.hologram.clear()
+
+    def add_image(self, path: str, _: str, x: int, y: int, width: int, height: int):
+        image = self.nvim.lua.hologram.new(path, x, y)
+        self.images.append(HologramImage(image, width=width, height=height))
+
+
+def get_canvas_given_provider(name: str, nvim: Nvim) -> Canvas:
     if name == "ueberzug":
         return UeberzugCanvas()
+    elif name == "hologram":
+        return Hologram(nvim)
     else:
         raise MagmaException(f"Unknown image provider: '{name}'")

@dccsillag
Copy link
Owner

Is this ready to be "merged", or do you want to work a bit more on it before that?

Also, could you elaborate a bit more on "hologram does not seem to be mature enough"?

@tzachar
Copy link
Contributor Author

tzachar commented Aug 17, 2021

Is this ready to be "merged", or do you want to work a bit more on it before that?
No its not. Im still not totally clear regarding the exact protocol which the Canvas class needs to implement. Can you add some documentation? this would greatly help.

Also, could you elaborate a bit more on "hologram does not seem to be mature enough"?
I managed to get hologram to display images, but removing them is still a bit clunky. Also, its not showing them consistently. Moreover, it does not work through tmux for now, which is also an issue.
I opened a pull request to handle some of the issues (edluffy/hologram.nvim#6), and will update on my progress...

Overall, I think the approach is sound, as the intention of hologram is to support kitty's graphics protocol, which is nice.

I am also considering other backends, like Noskcaj19/term-image, which also looks promising.

@dccsillag
Copy link
Owner

dccsillag commented Aug 17, 2021

Can you add some documentation? this would greatly help.

Yeah, of course! I didn't add anything much before because I figured it would change once we implemented things that weren't Ueberzug.

Is there something in particular which you want documented, or some particular way you want it documented? Right now I'm thinking of just adding some good docstrings to the Canvas class (and to the magma.image module, overall).

I managed to get hologram to display images, but removing them is still a bit clunky. Also, its not showing them consistently.

Ah, that's troublesome.

Moreover, it does not work through tmux for now, which is also an issue.

tmux, the bane of images in the terminal.

I opened a pull request to handle some of the issues (edluffy/hologram.nvim#6), and will update on my progress...

Cool!

Overall, I think the approach is sound, as the intention of hologram is to support kitty's graphics protocol, which is nice.

Yeah, it's a very nice and ambitious project.

I am also considering other backends

I'm also up for implementing some other backends. Might take a try at some soon.

[...], like Noskcaj19/term-image, which also looks promising.

It does look cool! Personally, I'm quite fond of https://github.com/atanunq/viu/, which also supports iTerm and Kitty protocols (as well as being able to show images composed by half-block characters, and some more cool stuff).

Unfortunatly, I suppose the way to implement a canvas for term-image or viu would be to use floating terminal windows and hope for support for specialized image protocols in neovim's terminal to be added. But there might be some other way.

@tzachar
Copy link
Contributor Author

tzachar commented Aug 17, 2021

I thinkmy next step is to implement native support for kitty.

@tzachar
Copy link
Contributor Author

tzachar commented Aug 17, 2021

As for documentation, I need to understand what exactly is expected for each method of the canvas, and which state needs to be maintained between calls.

@dccsillag
Copy link
Owner

Over on #15, https://github.com/dankamongmen/notcurses popped up; it seems like the perfect solution for a reliable image provider which supports these specialized image protocols. I'm currently taking a look at it.

@dccsillag
Copy link
Owner

I'll get the documentation done in a bit.

@dccsillag
Copy link
Owner

dccsillag commented Aug 17, 2021

Documentation is up. Please let me know if something is not clear.

@dccsillag
Copy link
Owner

Also, this abstraction is in no way finished, and probably should change. In particular, it seems rather unpractical to provide the path to an image instead of the actual image for non-Ueberzug backends (especially since Jupyter already gives us the image data, we are the ones creating the file) -- so the path: str in add_image should probably become some representation of image data (maybe a PIL image?).

@tzachar
Copy link
Contributor Author

tzachar commented Aug 18, 2021

See #16

@tzachar tzachar closed this as completed Aug 18, 2021
@dccsillag dccsillag added enhancement New feature or request output-chunks This is related to how we display some mimetype labels Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request output-chunks This is related to how we display some mimetype
Projects
None yet
Development

No branches or pull requests

2 participants