From 60a7059c3f352b252da3e825b5cfefd1352da952 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 14 Aug 2026 18:39:58 -0700 Subject: [PATCH] fix(terminal): align stored and fitted padding --- src/main/ghostty/mapper-extended.test.ts | 4 +- src/main/ghostty/numeric-config-values.ts | 7 +- .../TerminalWindowSection.padding.test.tsx | 81 +++++++++++++++++++ .../settings/TerminalWindowSection.tsx | 13 ++- .../terminal-pane/terminal-appearance.ts | 5 +- src/shared/terminal-padding-settings.test.ts | 14 ++++ src/shared/terminal-padding-settings.ts | 6 ++ 7 files changed, 119 insertions(+), 11 deletions(-) create mode 100644 src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx create mode 100644 src/shared/terminal-padding-settings.test.ts create mode 100644 src/shared/terminal-padding-settings.ts diff --git a/src/main/ghostty/mapper-extended.test.ts b/src/main/ghostty/mapper-extended.test.ts index 402e9253f21..189e5bf4217 100644 --- a/src/main/ghostty/mapper-extended.test.ts +++ b/src/main/ghostty/mapper-extended.test.ts @@ -90,9 +90,9 @@ describe('mapGhosttyToOrca — window-padding', () => { expect(result.unsupportedKeys).toEqual([]) }) - it('preserves odd-sum dual-value padding as a fractional average', () => { + it('rounds odd-sum dual-value padding to the nearest fitted pixel', () => { const result = mapGhosttyToOrca({ 'window-padding-x': '1,2' }) - expect(result.diff).toEqual({ terminalPaddingX: 1.5 }) + expect(result.diff).toEqual({ terminalPaddingX: 2 }) expect(result.unsupportedKeys).toEqual([]) }) diff --git a/src/main/ghostty/numeric-config-values.ts b/src/main/ghostty/numeric-config-values.ts index f8a7ce7b468..5ced69afa48 100644 --- a/src/main/ghostty/numeric-config-values.ts +++ b/src/main/ghostty/numeric-config-values.ts @@ -1,3 +1,5 @@ +import { normalizeTerminalPadding } from '../../shared/terminal-padding-settings' + // Why: `Number("1e10")` succeeds and passes `Number.isInteger`, so a Ghostty // config with `window-padding-x = 1e9` would sail through the mapper and land // an absurd value in the store. Restrict to plain decimal integers. @@ -11,8 +13,7 @@ export const parseStrictInt = (v: string): number | null => { } // Why: Ghostty accepts "top,bottom" / "left,right" pairs for window paddings, -// but Orca stores a single value per axis — average the pair so the total -// padding along the axis stays the same. +// but Orca stores an integer per axis — preserve the nearest total inset. export const parsePaddingValue = (v: string): number | null => { const parts = v.split(',') if (parts.length > 2) { @@ -26,5 +27,5 @@ export const parsePaddingValue = (v: string): number | null => { } nums.push(num) } - return nums.reduce((sum, num) => sum + num, 0) / nums.length + return normalizeTerminalPadding(nums.reduce((sum, num) => sum + num, 0) / nums.length) } diff --git a/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx b/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx new file mode 100644 index 00000000000..5f54fb38a9e --- /dev/null +++ b/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx @@ -0,0 +1,81 @@ +// @vitest-environment happy-dom + +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { GlobalSettings } from '../../../../shared/global-settings-types' +import { getDefaultSettings } from '../../../../shared/constants' +import { TerminalWindowSection } from './TerminalWindowSection' + +type NumberFieldProps = { + label: string + value: number + onChange: (value: number) => void +} + +const numberFields = vi.hoisted(() => new Map()) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, defaultValue: string) => defaultValue +})) + +vi.mock('./SettingsFormControls', () => ({ + ColorField: function ColorField() { + return null + }, + NumberField: function NumberField(props: NumberFieldProps) { + numberFields.set(props.label, props) + return null + } +})) + +vi.mock('./SearchableSetting', () => ({ + SearchableSetting: function SearchableSetting({ children }: { children?: ReactNode }) { + return children ?? null + } +})) + +vi.mock('../ui/button', () => ({ Button: () => null })) +vi.mock('../ui/label', () => ({ Label: () => null })) +vi.mock('../ui/switch', () => ({ Switch: () => null })) + +describe('TerminalWindowSection padding', () => { + let container: HTMLDivElement + let root: Root + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + numberFields.clear() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + document.body.replaceChildren() + }) + + it('displays and persists the same integer padding used by terminal fitting', () => { + const updateSettings = vi.fn() + const settings = { + ...getDefaultSettings('/tmp'), + terminalPaddingX: 1.5, + terminalPaddingY: 2.5 + } as GlobalSettings + + act(() => + root.render() + ) + + const horizontal = numberFields.get('Horizontal Padding') + const vertical = numberFields.get('Vertical Padding') + expect(horizontal?.value).toBe(2) + expect(vertical?.value).toBe(3) + + act(() => horizontal?.onChange(3.5)) + act(() => vertical?.onChange(4.5)) + expect(updateSettings).toHaveBeenNthCalledWith(1, { terminalPaddingX: 4 }) + expect(updateSettings).toHaveBeenNthCalledWith(2, { terminalPaddingY: 5 }) + }) +}) diff --git a/src/renderer/src/components/settings/TerminalWindowSection.tsx b/src/renderer/src/components/settings/TerminalWindowSection.tsx index 863d7fa11b0..5a09073cc27 100644 --- a/src/renderer/src/components/settings/TerminalWindowSection.tsx +++ b/src/renderer/src/components/settings/TerminalWindowSection.tsx @@ -1,6 +1,7 @@ import { useRef, useState } from 'react' import { RotateCw } from 'lucide-react' import type { GlobalSettings } from '../../../../shared/global-settings-types' +import { normalizeTerminalPadding } from '../../../../shared/terminal-padding-settings' import { Button } from '../ui/button' import { Label } from '../ui/label' import { Switch } from '../ui/switch' @@ -185,13 +186,15 @@ export function TerminalWindowSection({ 'Horizontal Padding' )} description="" - value={settings.terminalPaddingX ?? 4} + value={normalizeTerminalPadding(settings.terminalPaddingX ?? 4)} defaultValue={4} min={0} max={512} step={1} suffix="px" - onChange={(value) => updateSettings({ terminalPaddingX: Math.max(0, value) })} + onChange={(value) => + updateSettings({ terminalPaddingX: normalizeTerminalPadding(value) }) + } /> @@ -212,13 +215,15 @@ export function TerminalWindowSection({ 'Vertical Padding' )} description="" - value={settings.terminalPaddingY ?? 4} + value={normalizeTerminalPadding(settings.terminalPaddingY ?? 4)} defaultValue={4} min={0} max={512} step={1} suffix="px" - onChange={(value) => updateSettings({ terminalPaddingY: Math.max(0, value) })} + onChange={(value) => + updateSettings({ terminalPaddingY: normalizeTerminalPadding(value) }) + } /> diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.ts index 855557f3605..f520d98baba 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.ts @@ -3,6 +3,7 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { GlobalSettings } from '../../../../shared/global-settings-types' import { resolveTerminalFontWeights } from '../../../../shared/terminal-fonts' import { resolveTerminalLigaturesEnabled } from '../../../../shared/terminal-ligatures' +import { normalizeTerminalPadding } from '../../../../shared/terminal-padding-settings' import { getBuiltinTheme, resolvePaneStyleOptions, @@ -159,8 +160,8 @@ export function applyTerminalAppearance( settings.terminalFontFamily ) // FitAddon parses each CSS padding as an integer, so normalize imported half-pixels before styling and fitting. - const paddingX = Math.round(settings.terminalPaddingX ?? 4) - const paddingY = Math.round(settings.terminalPaddingY ?? 4) + const paddingX = normalizeTerminalPadding(settings.terminalPaddingX ?? 4) + const paddingY = normalizeTerminalPadding(settings.terminalPaddingY ?? 4) // Why before the pane loop: FitAddon subtracts live .xterm padding, so the // CSS vars must be stamped before safeFit or a padding shrink leaves a stale grid. diff --git a/src/shared/terminal-padding-settings.test.ts b/src/shared/terminal-padding-settings.test.ts new file mode 100644 index 00000000000..ee7ac432f46 --- /dev/null +++ b/src/shared/terminal-padding-settings.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from 'vitest' +import { normalizeTerminalPadding } from './terminal-padding-settings' + +describe('normalizeTerminalPadding', () => { + it('rounds fractional padding to the nearest fitted pixel', () => { + expect(normalizeTerminalPadding(1.5)).toBe(2) + expect(normalizeTerminalPadding(2.49)).toBe(2) + }) + + it('keeps padding inside the settings range', () => { + expect(normalizeTerminalPadding(-1)).toBe(0) + expect(normalizeTerminalPadding(513)).toBe(512) + }) +}) diff --git a/src/shared/terminal-padding-settings.ts b/src/shared/terminal-padding-settings.ts new file mode 100644 index 00000000000..1f18a46973f --- /dev/null +++ b/src/shared/terminal-padding-settings.ts @@ -0,0 +1,6 @@ +const MIN_TERMINAL_PADDING = 0 +const MAX_TERMINAL_PADDING = 512 + +export function normalizeTerminalPadding(value: number): number { + return Math.min(MAX_TERMINAL_PADDING, Math.max(MIN_TERMINAL_PADDING, Math.round(value))) +}