Skip to content

fix: add terminal font fallbacks#2150

Merged
arnestrickmann merged 2 commits into
mainfrom
jan/eng-1360-question-font-in-the-ui-has-lots-of-spacing-and-cant-change
May 20, 2026
Merged

fix: add terminal font fallbacks#2150
arnestrickmann merged 2 commits into
mainfrom
jan/eng-1360-question-font-in-the-ui-has-lots-of-spacing-and-cant-change

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

summary

  • adds fallback font
  • font quoting
  • tests

@janburzinski
Copy link
Copy Markdown
Collaborator Author

fixes #2142

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 20, 2026

Greptile Summary

This PR introduces terminal-font.ts, a small utility that normalises CSS font-family strings before they are handed to xterm: it splits comma-separated font lists while respecting quoted substrings, quotes each individual font name correctly, and appends a hardcoded set of terminal-safe fallbacks (Menlo, Monaco, Consolas, monospace). The new helper replaces all three previous raw-assignment sites in pty.ts and use-pty.ts.

  • terminal-font.ts — new splitFontFamilies (quote-aware comma splitter), quoteFontFamily (generic-family passthrough + already-quoted detection + backslash/double-quote escaping), and buildTerminalFontFamily (combines split, quote, dedup, fallback-append steps).
  • pty.ts — initial Terminal options now include fontFamily: buildTerminalFontFamily() so the fallback stack is active from construction.
  • use-pty.ts — settings-load and terminal-font-changed event paths both now call buildTerminalFontFamily(customFontFamily) instead of assigning raw user strings.

Confidence Score: 5/5

Safe to merge — the change is well-scoped, fully unit-tested, and correctly handles all the CSS font-family edge cases that were flagged in earlier review threads.

The new font utility correctly splits comma-separated font lists (respecting quoted substrings), quotes individual names, escapes embedded double-quotes, deduplicates against the fallback list, and handles CSS generic families. All three call sites in pty.ts and use-pty.ts are updated consistently. The unit tests cover the full range of inputs including pre-quoted CSS lists, embedded quotes, generic families, and the no-argument path. No regressions were found on the changed code paths.

No files require special attention.

Important Files Changed

Filename Overview
src/renderer/lib/pty/terminal-font.ts New module implementing CSS-aware font-family quoting, comma-aware splitting (respecting quoted substrings), and fallback appending. Logic and edge cases are correctly handled; all test cases verify correctly.
src/renderer/lib/pty/terminal-font.test.ts Comprehensive unit tests covering spaces, embedded quotes, comma-separated lists, pre-quoted CSS lists, generic families, and the no-argument fallback path.
src/renderer/lib/pty/pty.ts Adds buildTerminalFontFamily() to the initial xterm Terminal options so the fallback stack is active from construction, before any user settings are loaded.
src/renderer/lib/pty/use-pty.ts Both the settings-load path and the runtime font-change event path now route through buildTerminalFontFamily, ensuring fallbacks are always appended and font names are properly quoted.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[fontFamily string from user setting or event] --> B[buildTerminalFontFamily]
    B --> C{fontFamily provided?}
    C -- No --> D[TERMINAL_FONT_FALLBACKS only]
    C -- Yes --> E[splitFontFamilies]
    E --> F[Quote-aware comma split]
    F --> G[userFonts concat TERMINAL_FONT_FALLBACKS]
    D --> H[map quoteFontFamily over each name]
    G --> H
    H --> I{CSS generic family?}
    I -- Yes --> J[Return unquoted]
    I -- No --> K[Already quoted by regex?]
    K -- Yes --> L[Return as-is]
    K -- No --> M[Escape backslash and quotes then wrap in double quotes]
    J --> N[new Set deduplicate]
    L --> N
    M --> N
    N --> O[join with comma-space]
    O --> P[xterm fontFamily option]
Loading

Reviews (2): Last reviewed commit: "fix: preserve terminal font family lists" | Re-trigger Greptile

Comment thread src/renderer/lib/pty/terminal-font.ts
Comment thread src/renderer/lib/pty/terminal-font.ts Outdated
@janburzinski
Copy link
Copy Markdown
Collaborator Author

@greptileai

@arnestrickmann arnestrickmann merged commit 37fbfd9 into main May 20, 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