Skip to content

Edit ssh_config hosts, fix Nerd Font glyphs, and correct the README - #82

Merged
timhartmann7 merged 4 commits into
mainfrom
fix/gui-ssh-config-editing-and-fonts
Aug 20, 2026
Merged

Edit ssh_config hosts, fix Nerd Font glyphs, and correct the README#82
timhartmann7 merged 4 commits into
mainfrom
fix/gui-ssh-config-editing-and-fonts

Conversation

@timhartmann7

Copy link
Copy Markdown
Owner

Three of the feature requests in #69: editing ~/.ssh/config hosts (6), Nerd Font
glyphs (7), and the README line behind the batch-open request (3).

Refs #69

Deliberately not here: the ⌘T chord from request 4. ⌘K already opens a host list
whose Enter action on a host is spawnSession('terminal', …), and with no sessions
open the two palette modes render the same list — a second chord for the same window
is configurability without a feature behind it.

The README described things the app does not do

README.md:55 promised the desktop app "split the view" and "Open several servers at
once". Neither exists: SplitView is TUI-only (crates/omnyssh/src/app/terminal.rs:33)
and every spawn path is one host to one session. That line is what request 3 was
chasing — the reporter went looking for a feature the README invented.

Auditing the rest of the GUI section turned up four more of the same kind, so they are
fixed together:

  • :64 claimed ⌘K searches "Hosts, snippets, screens". paletteItems returns sessions
    and hosts only.
  • :58 claimed "Drag files across". There is no drag-and-drop in the SFTP pane — no
    dragstart/drop/dataTransfer handler exists in ui/src; transfers act on
    checkbox-marked entries.
  • :61 claimed a snippet runs "with a keypress" and can "broadcast to every server you
    have". There is no snippet chord (the GUI has three bindings: ⌘B, ⌘K, r) and no
    select-all — you tick hosts individually.
  • :81 claimed that after verifying the new key the app "offers to turn off password
    login". It does not offer. DisablePassword is step 4 of 6 in a fixed sequence
    (crates/omnyssh-core/src/ssh/key_setup.rs:707), with no confirmation anywhere in the
    core or either frontend. The ordering claim is true — verification really does come
    first — but a reader expects a prompt before losing password auth on a production box.
    Reworded to say the flow runs to the end once started. Same line also said "pick a
    password based host"; the button is gated on manual-source-and-no-key.

Nerd Font glyphs rendered as boxes

TerminalView.svelte asked xterm for a stack of system monospace families, none of
which carry the Private Use Area, so prompt glyphs fell to the webview's last-resort
font. Common Nerd Font families are now named as fallbacks in that stack and in
Tailwind's font-mono — the chrome renders raw remote output too (snippet results,
SFTP previews), which is the next place someone hits the same boxes.

Ordering is the whole design, and my first attempt got it wrong. Putting the patched
families behind the named system ones is not enough: those names are macOS/Windows
only, so on a Linux desktop the first family that resolves would have been
MesloLGS NF — a full text font, which would have taken over the app's Latin face and
sized the terminal cell from itself, for exactly the users this fix targets. They now
sit behind the generic monospace.

That relies on per-character fallback continuing past a generic family, so I measured
it rather than trusting the spec — via CDP getPlatformFontsForNode:

"漢" via [monospace]                               -> 蘋方-簡
"漢" via [monospace, "Apple SD Gothic Neo"]        -> Apple SD Gothic Neo
"漢" via [Menlo, monospace, "Apple SD Gothic Neo"] -> Apple SD Gothic Neo
"Aa" via [Menlo, monospace, "Apple SD Gothic Neo"] -> Menlo

The last two lines are the arrangement that shipped: a missing glyph reaches a family
declared after the generic, while Latin stays on the first concrete one.

Within the tail, single-width variants lead. Nerd Fonts v3 ships icons double-width in
the bare X Nerd Font family and one cell wide in its Mono twin, so … Nerd Font Mono
is named ahead of each bare family and Symbols Nerd Font Mono leads the whole tail.

The two stacks live in different files and would drift silently, so
nerdFontFallback.test.ts asserts both name every family and keep all of them behind
the generic. Mutation-tested: hoisting a family in either file, dropping the generic,
and deleting a family whose name is a prefix of another all fail it.

No font is bundled — you still need one installed.

Editing an ~/.ssh/config host

The GUI hid Edit for source === 'sshConfig'. The TUI has allowed the same edit since
before the GUI existed (crates/omnyssh/src/app/host.rs:474-492), and the core
machinery is already there — the GUI was the only frontend that never wired it up.

Editing an import now adopts it: save_host writes a manual entry of the same name
and merge_hosts already prefers a manual host over an import, so the parsed entry is
shadowed. ~/.ssh/config is never written; platform::ssh_config_path() still has
exactly one call site in the workspace and it is a read.

Enabling the button alone would have been a bug: upsert's new-host arm did not carry
proxy_jump or identity_file, and the form cannot see either (HostDto omits both,
§3.4) — an adopted bastion host would have started dialling its target address direct,
the hazard fixed in 1.1.1. save_host now takes State and recovers both from the
cached import, and records original_ssh_host the way the TUI does, so a copy later
renamed in the TUI still hides its import and still answers another host's ProxyJump
alias.

The editor says what saving does, since ~/.ssh/config stops reaching the host once it
is adopted. Delete stays manual-only, and its confirmation now warns that a same-named
SSH config entry comes back as an import. Key setup stays manual-only too, with a
comment saying why: it records its outcome through save_hosts, which keeps manual
entries only.

bindings.ts moves by doc comment only — specta erases State, so
saveHost(input: HostInputDto) is unchanged and committed_bindings_are_in_sync
passes. The e2e case that asserted imports are read-only encoded the old acceptance
criterion and was rewritten as an adoption round-trip.

tech-gui.md §4.1 and Stage 4.1 specified "manual entries only"; both are updated with
a resolved CONTRACT GAP note in the format §6.3 asks for.

Verification

cargo clippy --workspace --all-targets -- -D warnings, cargo fmt --all --check,
cargo test -p omnyssh-gui --locked (69 + 5), npm run check (418 files, 0/0),
npm test (219), npm run build, npx playwright test (28 passed) — clean. eslint and
knip are named as gates in CLAUDE.md but are not configured in this repo, so they were
not run.

Reviewed per CLAUDE.md: /code-review at high, then an adversarial pass. Both found
real defects — the font ordering above, a drift test that passed on a broken file, a
missing original_ssh_host, stale contract docs mirrored into bindings.ts, and the
four remaining README claims — and all are fixed here rather than waived.

Known, not fixed here

ssh_config.rs parses Host web prod-web as a single host literally named
"web prod-web" (it takes the rest of the line verbatim). Adoption lets that bogus name
be written into hosts.toml, where the form cannot fix it since the name is immutable
on edit. Pre-existing parser defect; this slice only widens its reach.

e2e/terminal.spec.ts:119 fails on main too — it expects the accessible name
Close web-1 · terminal while Sidebar.svelte renders Close web-1. CI does not run
the e2e suite. Left alone.

@timhartmann7
timhartmann7 force-pushed the fix/gui-ssh-config-editing-and-fonts branch from e84a006 to c21f958 Compare August 20, 2026 07:39
@timhartmann7
timhartmann7 merged commit b1dd164 into main Aug 20, 2026
6 checks passed
@timhartmann7
timhartmann7 deleted the fix/gui-ssh-config-editing-and-fonts branch August 20, 2026 07:49
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.

1 participant