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

Auto-detect Ghostty and Konsole as supporting the Kitty protocol #24

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

uncenter
Copy link
Contributor

@uncenter uncenter commented Aug 2, 2024

Follow up of #23, as per my comment: #10 (comment).

@lusingander
Copy link
Owner

Can you share that you actually have the application running? We need to make sure that scrolling, opening details, etc. works properly and then add it to the list of supported terminals.

https://github.com/lusingander/serie?tab=readme-ov-file#compatibility

@uncenter
Copy link
Contributor Author

uncenter commented Aug 2, 2024

Here's with Ghostty. I'll remove Konsole since I can't verify it works there, was just adding it to the list of terminals that support the Kitty protocol specifically since that is verifiable.

CleanShot.2024-08-02.at.08.41.19.mp4

@alerque
Copy link
Contributor

alerque commented Aug 2, 2024

I can verify Konsole works with this PR, see where HEAD is here, and I just used cargo run in a Konsole window:

image

@uncenter
Copy link
Contributor Author

uncenter commented Aug 2, 2024

Oop awesome, I'll revert that bit of my last commit then!

@uncenter uncenter changed the title Auto-detect other terminals supporting the Kitty protocol Auto-detect Ghostty and Konsole as supporting the Kitty protocol Aug 2, 2024
README.md Outdated Show resolved Hide resolved
@uncenter uncenter requested a review from alerque August 2, 2024 12:57
@uncenter
Copy link
Contributor Author

uncenter commented Aug 2, 2024

Hold up, made a mistake with Ghostty detection... Nevermind, forgot I switched branches 😅

@lusingander
Copy link
Owner

lusingander commented Aug 3, 2024

Thanks for checking, could you please share it on the Discussions?

I've created a Discussions page where we can share how the terminal works. It also contains information that we would like to know.

@lusingander
Copy link
Owner

Once Konsole has been shared on the Discussions or removed from the changes, I will merge this.

@alerque
Copy link
Contributor

alerque commented Aug 4, 2024

I already went out of my way to test Konsole. Not only does this PR properly detect it (which is the only aspect relevant to the PR in the first place) but while I had it installed and open I checked the visuals and usability and confirmed that it functions properly. I even posted a screen shot while I was at it. I'm not going back and record video, sorry—not sorry. Don't put undue burdens on the contribution process. Clearly what this PR does relevant to Konsole is correct. The terminal is already supported, this just detects which protocol to use as for other terminals which is better than not working at all unless people know to add an extra argument.

@uncenter
Copy link
Contributor Author

Would love to get this merged - @lusingander are we alright with Konsole support as demonstrated by @alerque?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants