Revert "fix(terminal): apply pane padding on all four edges (#15544)" (#15623)

This reverts commit 4b2ed5ddd4.
This commit is contained in:
Jinjing
2026-08-20 09:57:55 -07:00
committed by GitHub
parent 0f26ff4ad8
commit d8e9fa1bb9
12 changed files with 37 additions and 456 deletions
+2 -2
View File
@@ -90,9 +90,9 @@ describe('mapGhosttyToOrca — window-padding', () => {
expect(result.unsupportedKeys).toEqual([])
})
it('rounds odd-sum dual-value padding to the integer used for fitting', () => {
it('preserves odd-sum dual-value padding as a fractional average', () => {
const result = mapGhosttyToOrca({ 'window-padding-x': '1,2' })
expect(result.diff).toEqual({ terminalPaddingX: 2 })
expect(result.diff).toEqual({ terminalPaddingX: 1.5 })
expect(result.unsupportedKeys).toEqual([])
})
+4 -5
View File
@@ -1,5 +1,3 @@
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.
@@ -12,8 +10,9 @@ export const parseStrictInt = (v: string): number | null => {
return Number.isFinite(num) ? num : null
}
// Why: Orca stores one integer per axis, so collapse paired paddings to the
// nearest value that xterm's integer-based fit calculation will also use.
// 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.
export const parsePaddingValue = (v: string): number | null => {
const parts = v.split(',')
if (parts.length > 2) {
@@ -27,5 +26,5 @@ export const parsePaddingValue = (v: string): number | null => {
}
nums.push(num)
}
return normalizeTerminalPadding(nums.reduce((sum, num) => sum + num, 0) / nums.length)
return nums.reduce((sum, num) => sum + num, 0) / nums.length
}
@@ -5,7 +5,9 @@ const terminalCss = fs.readFileSync(new URL('./terminal.css', import.meta.url),
describe('terminal container geometry', () => {
it('keeps the hidden link tooltip out of the fitted terminal height', () => {
expect(terminalCss).toMatch(/\.xterm-container\s*{[^}]*height:\s*100%;/s)
expect(terminalCss).toMatch(
/\.xterm-container\s*{[^}]*height:\s*calc\(100% - var\(--pane-padding-y, 4px\)\);/s
)
expect(terminalCss).toMatch(
/\.pane\[data-has-title\] \.xterm-container\s*{[^}]*height:\s*calc\(100% - var\(--orca-pane-title-height\)\);/s
)
@@ -13,12 +15,4 @@ describe('terminal container geometry', () => {
/\.pane-link-tooltip\s*{[^}]*height:\s*var\(--orca-terminal-link-tooltip-height\);/s
)
})
it('insets the xterm grid on all four edges', () => {
expect(terminalCss).toMatch(
/\.xterm-container \.xterm\s*{[^}]*padding:\s*var\(--pane-padding-y, 4px\) var\(--pane-padding-x, 4px\);/s
)
expect(terminalCss).toMatch(/\.xterm-container\s*{[^}]*width:\s*100%;/s)
expect(terminalCss).not.toMatch(/\.xterm-container\s*{[^}]*margin-left:/s)
})
})
+9 -8
View File
@@ -25,11 +25,9 @@
outline: none;
}
.xterm-container .xterm {
box-sizing: border-box;
.pane-manager-root .xterm {
height: 100%;
width: 100%;
padding: var(--pane-padding-y, 4px) var(--pane-padding-x, 4px);
}
.pane-manager-root .xterm-viewport {
@@ -179,7 +177,7 @@
.pane[data-terminal-attention] .xterm-container::after {
content: '';
position: absolute;
inset: 0;
inset: 0 4px 4px 0;
z-index: 5;
pointer-events: none;
border-radius: 2px;
@@ -500,13 +498,16 @@
border-color: var(--ring);
}
/* The container fills the pane; xterm owns the four-edge grid inset so its fit
calculation subtracts the configured padding before choosing cols/rows. */
/* Baseline xterm-container layout — padding is driven by CSS custom properties
so Ghostty-imported window-padding-x/y can be applied without inline styles.
The fallback 4px preserves the legacy default. */
.xterm-container {
box-sizing: border-box;
position: relative;
width: 100%;
height: 100%;
width: calc(100% - var(--pane-padding-x, 4px));
height: calc(100% - var(--pane-padding-y, 4px));
margin-top: var(--pane-padding-y, 4px);
margin-left: var(--pane-padding-x, 4px);
}
/* When a pane has a title, shift the terminal content down to make room.
@@ -1,81 +0,0 @@
// @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<string, NumberFieldProps>())
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(<TerminalWindowSection settings={settings} updateSettings={updateSettings} />)
)
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 })
})
})
@@ -1,6 +1,5 @@
import { useRef, useState } from 'react'
import { RotateCw } from 'lucide-react'
import { normalizeTerminalPadding } from '../../../../shared/terminal-padding-settings'
import type { GlobalSettings } from '../../../../shared/global-settings-types'
import { Button } from '../ui/button'
import { Label } from '../ui/label'
@@ -186,15 +185,13 @@ export function TerminalWindowSection({
'Horizontal Padding'
)}
description=""
value={normalizeTerminalPadding(settings.terminalPaddingX ?? 4)}
value={settings.terminalPaddingX ?? 4}
defaultValue={4}
min={0}
max={512}
step={1}
suffix="px"
onChange={(value) =>
updateSettings({ terminalPaddingX: normalizeTerminalPadding(value) })
}
onChange={(value) => updateSettings({ terminalPaddingX: Math.max(0, value) })}
/>
</SearchableSetting>
@@ -215,15 +212,13 @@ export function TerminalWindowSection({
'Vertical Padding'
)}
description=""
value={normalizeTerminalPadding(settings.terminalPaddingY ?? 4)}
value={settings.terminalPaddingY ?? 4}
defaultValue={4}
min={0}
max={512}
step={1}
suffix="px"
onChange={(value) =>
updateSettings({ terminalPaddingY: normalizeTerminalPadding(value) })
}
onChange={(value) => updateSettings({ terminalPaddingY: Math.max(0, value) })}
/>
</SearchableSetting>
@@ -3,7 +3,6 @@ import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager'
import { getDefaultSettings } from '../../../../shared/constants'
import {
applyTerminalAppearance,
composeRendererTerminalTheme,
hexToRgba,
publishTerminalViewAttributesAtAppStart
} from './terminal-appearance'
@@ -217,54 +216,6 @@ describe('applyTerminalAppearance theme assignment', () => {
expect(pane.terminal.options.theme?.background).toBe('#102030')
})
it('lets the pane paint a fractional background exactly once', () => {
const pane = makePane(1)
const settings = getDefaultSettings('/tmp')
const manager = makeManager([pane])
applyTerminalAppearance(
manager,
{ ...settings, terminalBackgroundOpacity: 0.5 },
true,
new Map(),
new Map(),
'false',
new Map(),
new Map()
)
expect(pane.terminal.options.theme?.background).toMatch(/^rgba\(.+, 0\)$/)
expect(pane.terminal.options.allowTransparency).toBe(true)
expect(manager.setPaneStyleOptions).toHaveBeenCalledWith(
expect.objectContaining({ paneBackground: expect.stringMatching(/^rgba\(.+, 0\.5\)$/) })
)
})
it('stamps four-edge padding before fitting', () => {
const pane = makePane(1)
pane.fitAddon.proposeDimensions = vi.fn(() => ({ cols: 79, rows: 23 }))
const manager = makeManager([pane])
const settings = getDefaultSettings('/tmp')
applyTerminalAppearance(
manager,
{ ...settings, terminalPaddingX: 11, terminalPaddingY: 7 },
true,
new Map(),
new Map(),
'false',
new Map(),
new Map()
)
expect(manager.setPaneStyleOptions).toHaveBeenCalledWith(
expect.objectContaining({ paddingX: 11, paddingY: 7 })
)
expect(vi.mocked(manager.setPaneStyleOptions).mock.invocationCallOrder[0]).toBeLessThan(
vi.mocked(pane.fitAddon.proposeDimensions).mock.invocationCallOrder[0]!
)
})
// #7934: contrast correction rescues invisible white text on light backgrounds but over-corrects on dark;
// gate by the composed theme's background luminance (either theme slot can hold either kind of theme).
it('keeps xterm contrast correction on light themes', () => {
@@ -411,23 +362,6 @@ describe('applyTerminalAppearance theme assignment', () => {
})
})
describe('composeRendererTerminalTheme', () => {
it('preserves background RGB while removing fractional renderer alpha', () => {
const theme = { background: 'rgba(16, 32, 48, 0.5)', foreground: '#ffffff' }
expect(composeRendererTerminalTheme(theme, 0.5)).toEqual({
...theme,
background: 'rgba(16, 32, 48, 0)'
})
})
it('keeps the opaque fast path unchanged', () => {
const theme = { background: '#102030' }
expect(composeRendererTerminalTheme(theme, 1)).toBe(theme)
})
})
describe('publishTerminalViewAttributesAtAppStart', () => {
// Hidden-at-launch PTYs query OSC 10/11 before any pane mounts; publish with no pane manager (terminal-query-authority.md).
it('publishes composed attributes without any pane mount and dedupes repeats', () => {
@@ -29,7 +29,6 @@ import { publishTerminalViewAttributes } from './terminal-view-attributes-publis
import { normalizeTerminalLineHeight } from '../../../../shared/terminal-line-height-settings'
import { maybePushMode2031Flip } from './terminal-mode-2031-replies'
import { resolveTerminalMinimumContrastRatio } from '@/lib/terminal-contrast-correction'
import { normalizeTerminalPadding } from '../../../../shared/terminal-padding-settings'
export function hexToRgba(hex: string, alpha: number): string {
let clean = hex.replace('#', '')
@@ -133,22 +132,6 @@ function composedTerminalThemesEqual(a: ITheme | undefined, b: ITheme): boolean
return extA.length === extB.length && extA.every((value, i) => value === extB[i])
}
export function composeRendererTerminalTheme(
theme: ITheme | null,
backgroundOpacity: number | undefined
): ITheme | null {
if (!theme?.background || backgroundOpacity === undefined || backgroundOpacity >= 1) {
return theme
}
const transparentBackground = theme.background.replace(
/^rgba\(([^,]+),\s*([^,]+),\s*([^,]+),\s*[^)]+\)$/,
'rgba($1, $2, $3, 0)'
)
return transparentBackground === theme.background
? theme
: { ...theme, background: transparentBackground }
}
export function applyTerminalAppearance(
manager: PaneManager,
settings: GlobalSettings,
@@ -166,7 +149,6 @@ export function applyTerminalAppearance(
// Publish composed appearance to main's hidden-PTY query responder — the only point it exists; deduped in the publisher.
publishTerminalViewAttributes(theme, appearance.mode, settings)
const paneBackground = theme?.background ?? '#000000'
const rendererTheme = composeRendererTerminalTheme(theme, settings.terminalBackgroundOpacity)
const terminalFontWeights = resolveTerminalFontWeights(
settings.terminalFontWeight,
@@ -177,23 +159,10 @@ export function applyTerminalAppearance(
settings.terminalFontFamily
)
// Why before fit: FitAddon reads live .xterm padding when proposing cols/rows.
manager.setPaneStyleOptions({
splitBackground: paneBackground,
paneBackground,
inactivePaneOpacity: paneStyles.inactivePaneOpacity,
activePaneOpacity: paneStyles.activePaneOpacity,
opacityTransitionMs: paneStyles.opacityTransitionMs,
dividerThicknessPx: paneStyles.dividerThicknessPx,
focusFollowsMouse: paneStyles.focusFollowsMouse,
paddingX: normalizeTerminalPadding(settings.terminalPaddingX ?? 4),
paddingY: normalizeTerminalPadding(settings.terminalPaddingY ?? 4)
})
for (const pane of manager.getPanes()) {
// Why value-gated: writing options.theme rebuilds the palette, discarding TUI OSC 4/10/11/12 mutations; skip on no-op change.
if (rendererTheme && !composedTerminalThemesEqual(pane.terminal.options.theme, rendererTheme)) {
pane.terminal.options.theme = rendererTheme
if (theme && !composedTerminalThemesEqual(pane.terminal.options.theme, theme)) {
pane.terminal.options.theme = theme
}
// Gate off the configured theme background; the live OSC-11 background is deliberately preserved by the
// theme write above, so a TUI that repaints its background at runtime won't re-gate (known limitation).
@@ -258,4 +227,16 @@ export function applyTerminalAppearance(
safeFit(pane)
}
}
manager.setPaneStyleOptions({
splitBackground: paneBackground,
paneBackground,
inactivePaneOpacity: paneStyles.inactivePaneOpacity,
activePaneOpacity: paneStyles.activePaneOpacity,
opacityTransitionMs: paneStyles.opacityTransitionMs,
dividerThicknessPx: paneStyles.dividerThicknessPx,
focusFollowsMouse: paneStyles.focusFollowsMouse,
paddingX: settings.terminalPaddingX,
paddingY: settings.terminalPaddingY
})
}
@@ -1,14 +0,0 @@
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)
})
})
-6
View File
@@ -1,6 +0,0 @@
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)))
}