Edit ssh_config hosts, fix Nerd Font glyphs, and correct the README - #82
Merged
Conversation
timhartmann7
force-pushed
the
fix/gui-ssh-config-editing-and-fonts
branch
from
August 20, 2026 07:39
e84a006 to
c21f958
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three of the feature requests in #69: editing
~/.ssh/confighosts (6), Nerd Fontglyphs (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 sessionsopen 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:55promised the desktop app "split the view" and "Open several servers atonce". Neither exists:
SplitViewis 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:
:64claimed ⌘K searches "Hosts, snippets, screens".paletteItemsreturns sessionsand hosts only.
:58claimed "Drag files across". There is no drag-and-drop in the SFTP pane — nodragstart/drop/dataTransferhandler exists inui/src; transfers act oncheckbox-marked entries.
:61claimed a snippet runs "with a keypress" and can "broadcast to every server youhave". There is no snippet chord (the GUI has three bindings: ⌘B, ⌘K,
r) and noselect-all — you tick hosts individually.
:81claimed that after verifying the new key the app "offers to turn off passwordlogin". It does not offer.
DisablePasswordis step 4 of 6 in a fixed sequence(
crates/omnyssh-core/src/ssh/key_setup.rs:707), with no confirmation anywhere in thecore 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.svelteasked xterm for a stack of system monospace families, none ofwhich 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 andsized 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: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 Fontfamily and one cell wide in itsMonotwin, so… Nerd Font Monois named ahead of each bare family and
Symbols Nerd Font Monoleads the whole tail.The two stacks live in different files and would drift silently, so
nerdFontFallback.test.tsasserts both name every family and keep all of them behindthe 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/confighostThe GUI hid Edit for
source === 'sshConfig'. The TUI has allowed the same edit sincebefore the GUI existed (
crates/omnyssh/src/app/host.rs:474-492), and the coremachinery is already there — the GUI was the only frontend that never wired it up.
Editing an import now adopts it:
save_hostwrites a manual entry of the same nameand
merge_hostsalready prefers a manual host over an import, so the parsed entry isshadowed.
~/.ssh/configis never written;platform::ssh_config_path()still hasexactly 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 carryproxy_jumporidentity_file, and the form cannot see either (HostDtoomits both,§3.4) — an adopted bastion host would have started dialling its target address direct,
the hazard fixed in 1.1.1.
save_hostnow takesStateand recovers both from thecached import, and records
original_ssh_hostthe way the TUI does, so a copy laterrenamed in the TUI still hides its import and still answers another host's
ProxyJumpalias.
The editor says what saving does, since
~/.ssh/configstops reaching the host once itis 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 manualentries only.
bindings.tsmoves by doc comment only — specta erasesState, sosaveHost(input: HostInputDto)is unchanged andcommitted_bindings_are_in_syncpasses. 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 witha resolved
CONTRACT GAPnote 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 andknip are named as gates in CLAUDE.md but are not configured in this repo, so they were
not run.
Reviewed per CLAUDE.md:
/code-reviewat high, then an adversarial pass. Both foundreal defects — the font ordering above, a drift test that passed on a broken file, a
missing
original_ssh_host, stale contract docs mirrored intobindings.ts, and thefour remaining README claims — and all are fixed here rather than waived.
Known, not fixed here
ssh_config.rsparsesHost web prod-webas a single host literally named"web prod-web"(it takes the rest of the line verbatim). Adoption lets that bogus namebe written into
hosts.toml, where the form cannot fix it since the name is immutableon edit. Pre-existing parser defect; this slice only widens its reach.
e2e/terminal.spec.ts:119fails onmaintoo — it expects the accessible nameClose web-1 · terminalwhileSidebar.svelterendersClose web-1. CI does not runthe e2e suite. Left alone.