Skip to content

feat: add termy terminal#2116

Merged
arnestrickmann merged 2 commits into
mainfrom
emdash/termy-add-wr8q2
May 19, 2026
Merged

feat: add termy terminal#2116
arnestrickmann merged 2 commits into
mainfrom
emdash/termy-add-wr8q2

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

termy.sh

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 19, 2026

Greptile Summary

This PR adds Termy (termy.sh) as a supported "open in" terminal app, registering it across macOS, Windows, and Linux via URL scheme (termy://new?dir=…) with a new icon asset.

  • The entry is correctly wired into ICON_PATHS and _OPEN_IN_APPS, so type-safety and detection logic (checkInstalledApps) will pick it up automatically.
  • All three platform configs define both openUrls and openCommands, but openInLocal in service.ts exclusively uses openUrls when that field is present — the openCommands entries are never reached and provide no fallback if the termy:// URL scheme is not registered.

Confidence Score: 3/5

The launch path for Termy falls back to an error rather than the openCommands fallbacks whenever the termy:// URL scheme is absent.

The openCommands entries across all three platforms appear to be intentional fallbacks for when the URL scheme is not registered, but openInLocal never reaches them — it throws instead. A user with Termy installed via CLI but without the URL scheme handler registered will get an error on every platform.

src/shared/openInApps.ts — the Termy platform configs need the openCommands entries either removed (if the URL scheme is always guaranteed) or the openUrls entries removed so the commands-based path is used.

Important Files Changed

Filename Overview
src/shared/openInApps.ts Adds Termy terminal app entry; openCommands are unreachable dead code on all three platforms because openInLocal exclusively uses openUrls when that field is present, with no fallback to commands.
src/assets/images/termy.png New icon asset for the Termy terminal app; no issues found.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[openIn called with app=termy] --> B[openInLocal]
    B --> C{platformConfig.openUrls defined?}
    C -->|Yes - all 3 platforms| D[Try termy://new?dir=path_url]
    D -->|shell.openExternal succeeds| E[✅ Termy opens in correct dir]
    D -->|shell.openExternal throws| F[❌ Throw: URI scheme not registered]
    F --> G[openCommands never reached]
    C -->|No| H[Try openCommands with cwd=target]
    style G fill:#f99,stroke:#c00
    style F fill:#f99,stroke:#c00
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/shared/openInApps.ts:288-310
**`openCommands` are unreachable dead code on all three platforms**

`openInLocal` in `service.ts` checks `if (platformConfig?.openUrls)` first and, when that array is present, iterates through every URL then either returns or throws — it never falls through to `openCommands` (lines 337–352 of `service.ts`). Because Termy's darwin, win32, and linux configs all define `openUrls`, the `openCommands` entries will never execute. No other app in the registry mixes both fields for the same platform; warp (the closest analogue) defines only `openUrls` on darwin.

In practice this means: if the `termy://` URL scheme is not registered on a user's machine (e.g. the CLI is installed but the URL handler isn't), the launch attempt will throw `"Termy is not installed or its URI scheme is not registered on this platform."` with no fallback — even though `openCommands` entries exist and appear to handle that case.

Reviews (1): Last reviewed commit: "Add Termy open-in integration" | Re-trigger Greptile

Comment thread src/shared/openInApps.ts
@janburzinski
Copy link
Copy Markdown
Collaborator Author

doing it like ghostty does it now (lasse fixed that in termy directly :D)

Copy link
Copy Markdown
Contributor

@arnestrickmann arnestrickmann left a comment

Choose a reason for hiding this comment

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

Lasse will be happy

@arnestrickmann arnestrickmann merged commit f35db7f into main May 19, 2026
1 check passed
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.

2 participants