Skip to content

Add Ghostty terminal hyperlink support#3

Open
the-spectator wants to merge 4 commits into
piotrmurach:masterfrom
the-spectator:feat/ghostty
Open

Add Ghostty terminal hyperlink support#3
the-spectator wants to merge 4 commits into
piotrmurach:masterfrom
the-spectator:feat/ghostty

Conversation

@the-spectator

Copy link
Copy Markdown

Closes #2

Describe the change

Add Ghostty terminal hyperlink support

Why are we doing this?

To extend the supported terminal list.

Requirements

  • Tests written & passing locally?
  • Code style checked?
  • Rebased with master branch?
  • Documentation updated?
  • Changelog updated?

@piotrmurach piotrmurach left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hi Akshay,

Thank you for submitting this PR. 🙏

I'm all in favour of adding Ghostty support. I left a couple of comments, though. In particular, I'm curious about the version check. Would you have time to investigate?

Comment thread CHANGELOG.md Outdated
Comment thread lib/tty/link/terminals/ghostty.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
@the-spectator

Copy link
Copy Markdown
Author

Thanks for the review @piotrmurach ! I have updated the code as per review comments.

@piotrmurach piotrmurach left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for making the changes. We are nearly there.

Comment thread CHANGELOG.md Outdated
Comment thread lib/tty/link/terminals/ghostty.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
@the-spectator the-spectator force-pushed the feat/ghostty branch 2 times, most recently from 5f9fbed to f7ab9bb Compare December 16, 2025 08:45
@the-spectator

Copy link
Copy Markdown
Author

Friendly bump for review.

@piotrmurach piotrmurach left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@the-spectator Sorry for the delay in my review.

I went through all the changes. Most comments relate to tests to ensure consistency in formatting and ordering. I included an extra test to match the other terminal tests.

Once this is merged, I plan to make a couple more changes to link rendering before making a new release.

Comment thread spec/unit/terminals/ghostty_spec.rb
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/link_spec.rb Outdated
Comment thread CHANGELOG.md Outdated
Comment thread lib/tty/link/terminals/ghostty.rb Outdated
@the-spectator

Copy link
Copy Markdown
Author

Thanks for the review @piotrmurach ! I have addressed the review comments.

@piotrmurach piotrmurach left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for making the changes. I noticed some small test improvements.

Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/terminals/ghostty_spec.rb Outdated
Comment thread spec/unit/link_spec.rb Outdated
@piotrmurach

Copy link
Copy Markdown
Owner

@the-spectator Sorry to keep you waiting so long for a review. I appreciate your patience. I plan to resume making updates soon. Please do not rebase until further notice. I may backtrack on some of the changes. Once done, I will let you know so we can merge this PR.

@the-spectator

Copy link
Copy Markdown
Author

Noted. 👍

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.

Supporting Ghostty Terminal

2 participants