LazoCoder / Pokemon-Terminal

Pokemon terminal themes.
GNU General Public License v3.0
4.2k stars 225 forks source link

Kitty support #207

Closed carlosvigil closed 2 months ago

carlosvigil commented 1 year ago

Is it possible to add or build kitty support? How would I go about that?

sylveon commented 1 year ago

You would need to add an adapter in https://github.com/LazoCoder/Pokemon-Terminal/tree/master/pokemonterminal/terminal/adapters

carlosvigil commented 1 year ago

Kitty uses PNG, meaning I might require image magick or something similar to convert the files. I haven't found an original image source, looks like their servers are down.

Edit: Think I'll be switching to warp terminal. If anyone wanted to start it: looks like it uses a yaml with the image address https://github.com/warpdotdev/themes

sylveon commented 1 year ago

No PNG support at all? That's interesting

cnschn commented 3 months ago

I'm willing to give this a go, I think apart from Kitty only supporting PNG for backgrounds it should be pretty easy to add.

What would the preferred way to go be here, I see two things to decide:

  1. How to convert jpg -> png: a) Call out to imagemagick, basically adding it as an optional dependency if you want to use kitty b) Add pillow as a dependency for this project and do the conversion in python (probably not optional? Not sure how this would complicate packaging)

  2. When to convert: a) Convert all images in bulk the first time pokemon-terminal is run inside kitty b) On the fly for each selected image, if it's not converted yet

In any case I'd store the converted images somewhere in ~/.cache.

I'm tending slightly towards 1-a and 2-b, any thoughts?

sylveon commented 2 months ago

1-a 2-b sounds like what I'd do as well!

cnschn commented 2 months ago

Huh. I just tried, and I can set jpg backgrounds just fine :shrug: Not sure if that's some unspecified behavior that randomly works, because the last comment I saw by the kitty developer on this topic still said that only png was supported. I'll go take a look at the kitty source if I can find a definitive answer, but that would make the kitty integration pretty simple (I have it working right now).

There is one unfortunate thing though - to dynamically set the background for a kitty instance, we need to use the remote control feature. This needs to be enabled in the kitty config, but there are a bunch of different ways to go about it (over TTY only, over a socket, with or without password protection (which is probably a good idea to have)). The easiest way to go would be to say "if you want to use Pokemon-Terminal with kitty, you need to fully enable remote control without a password" but I don't think I'd like this approach :smile:

The best thing would probably be to direct the user to setup a specific password for Pokemon-Terminal to use that only grants permissions to set the background, this would look like:

remote_control_password "missingno" set-background-image

Unfortunately this would need some configuration for Pokemon-Terminal, to tell the kitty adapter which password to use, either using a config file, environment variables or additional parameters... Any thoughts?

sylveon commented 2 months ago

Kitty documents that the CLI tool defaults to reading a passwords from the KITTY_RC_PASSWORD environment variable - so it seems to me that the easiest way to get is to just get users to set this themselves. Thoughts?

cnschn commented 2 months ago

Oh, I like that a lot! Then we can just put a note in the readme that kitty support needs remote control permissions to execute set-background-image, and it's up to the user to make that happen. Allowing RC without password would be one (not great) way, setting up a password and passing it in the environment variable another one.

I'll check if this works as expected and whip up a PR!

Silly sidenote: If I were to add a line to the readme about what needs to be put in the kitty config, do you think "missingno" as an example password would be fine, or is "" a safer choice? I just found it really funny when typing the comment above 😅

sylveon commented 2 months ago

Haha yeah missingno works fine as example password

cnschn commented 2 months ago

It's bizarre, I'm not familiar with kitty's codebase, but it really does look like it should only support PNG files. Still, #214 just works for me? I opened a discussion in the kitty project, let's wait for some feedback there. In the meantime, I'd be glad if anyone using kitty can give this a try. Maybe @carlosvigil is still interested, a good year later? :smile: