Skip to content
This repository has been archived by the owner on Mar 31, 2024. It is now read-only.

Add support for hyperlinks in kitty #165

Closed
wants to merge 1 commit into from
Closed

Add support for hyperlinks in kitty #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 17 additions & 4 deletions src/terminal/kitty.rs
View file
Open in desktop
Expand Up @@ -32,11 +32,24 @@ use std::process::{Command, Stdio};
use std::str;
use url::Url;

/// Whether we run in Kitty or not.
pub fn is_kitty() -> bool {
/// Whether we run in Kitty or not. It also returns the version.
pub fn is_kitty() -> Option<(u8, u8, u8)> {
std::env::var("TERM")
.map(|value| value == "xterm-kitty")
.unwrap_or(false)
.ok()
.filter(|value| value == "xterm-kitty")?;

let output = Command::new("kitty").arg("--version").output().ok()?;
Copy link
Owner

@swsnr swsnr Oct 6, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail if kitty is not in $PATH or doesn't exist at all (e.g. inside a chroot or systemd container). And worse, it won't fail gracefully in this case and detect kitty without hyperlinks; instead it'll make mdcat believe that it's not running in kitty at all, even if $TERM says it's kitty.

Can we somehow get the version in a more reliable way? Perhaps by means of some escape sequence?

And is the version actually sufficient? What about the allow_hyperlinks setting?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If kitty is not in $PATH image display would also fail. You're right about allow_hyperlinks, but looking at the issue and at the commits in kitty I haven't seen anything implemented for feature detection.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware that image display uses kitty. In this case I agree that it's a good idea to fail completely if kitty isn't found! But please document this in the docstring of the method, so that I don't forget and think it's a bug in six months 😉

if output.status.success() {
// Output is in the form of `kitty <major>.<minor>.<patch> created...`.
let output = std::str::from_utf8(&output.stdout).ok()?;
let mut version = output.split_ascii_whitespace().nth(1)?.split('.');
let major = version.next()?.parse().ok()?;
let minor = version.next()?.parse().ok()?;
let patch = version.next()?.parse().ok()?;
Some((major, minor, patch))
} else {
None
}
}

/// Retrieve the terminal size in pixels by calling the command-line tool `kitty`.
Expand Down
12 changes: 8 additions & 4 deletions src/terminal/mod.rs
View file
Open in desktop
Expand Up @@ -128,11 +128,15 @@ impl TerminalCapabilities {
}

/// Terminal capabilities of Kitty.
pub fn kitty() -> TerminalCapabilities {
pub fn kitty(version: (u8, u8, u8)) -> TerminalCapabilities {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like for these constructors to always return the same set of capabilities.

Would you mind to leave pub fn kitty() as is, and add a separate and independent pub fn kitty_with_links()? And the move the condition either into a separate pub fn kitty_autodetect(version) or even just to detect()?

TerminalCapabilities {
name: "Kitty".to_string(),
style: Some(StyleCapability::Ansi(AnsiStyle)),
links: None,
links: if version >= (0, 19, 0) {
Some(LinkCapability::OSC8(self::osc::OSC8Links))
} else {
None
},
image: Some(ImageCapability::Kitty(self::kitty::KittyImages)),
marks: None,
}
Expand All @@ -155,8 +159,8 @@ impl TerminalCapabilities {
Self::iterm2()
} else if self::terminology::is_terminology() {
Self::terminology()
} else if self::kitty::is_kitty() {
Self::kitty()
} else if let Some(version) = self::kitty::is_kitty() {
Self::kitty(version)
} else if get_vte_version().filter(|&v| v >= (50, 0)).is_some() {
Self::vte50()
} else {
Expand Down