diff --git a/e2e/copy-side-themes.spec.mjs b/e2e/copy-side-themes.spec.mjs new file mode 100644 index 00000000..5ad9b188 --- /dev/null +++ b/e2e/copy-side-themes.spec.mjs @@ -0,0 +1,98 @@ +import { test, expect, openSettings, stubOpenDialog } from './fixtures.mjs' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { THEMES } from '../src/renderer/src/utils/themes.js' + +// The slot's copy control is the one surface theme-sweep cannot reach: every +// surface there is opened through paste mode, which replaces the slots row, and +// the control only exists once a real FILE is loaded. So the all-20 check lives +// here instead, measuring the same thing the sweep would — the computed colours +// off the live DOM, composited against what is actually behind them. + +// It carries an icon, not prose: --text-dim at the mark floor, the same one +// check-theme-depth holds dim ink to. +const DIM = 3.0 + +const contrastOf = (page, selector) => + page.evaluate((sel) => { + const parse = (c) => (c.match(/[\d.]+/g) ?? []).map(Number) + const over = (fg, bg) => { + const a = fg.length > 3 ? fg[3] : 1 + return [0, 1, 2].map((i) => fg[i] * a + bg[i] * (1 - a)) + } + const lum = (rgb) => { + const [r, g, b] = rgb.map((v) => { + const s = v / 255 + return s <= 0.03928 ? s / 12.92 : ((s + 0.055) / 1.055) ** 2.4 + }) + return 0.2126 * r + 0.7152 * g + 0.0722 * b + } + // Walk up compositing every translucent layer, or a veil like --btn-face + // measures against nothing and reads as perfect contrast. + const groundOf = (el) => { + let acc = [255, 255, 255] + const chain = [] + for (let n = el; n; n = n.parentElement) chain.unshift(n) + for (const n of chain) { + const bg = parse(getComputedStyle(n).backgroundColor) + if (bg.length >= 3 && (bg.length < 4 || bg[3] > 0)) acc = over(bg, acc) + } + return acc + } + const el = document.querySelector(sel) + if (!el) return null + const ground = groundOf(el.parentElement ?? el) + const surface = (() => { + const own = parse(getComputedStyle(el).backgroundColor) + return own.length >= 3 && (own.length < 4 || own[3] > 0) ? over(own, ground) : ground + })() + const ink = over(parse(getComputedStyle(el).color), surface) + const [a, b] = [lum(ink), lum(surface)].sort((x, y) => y - x) + return (a + 0.05) / (b + 0.05) + }, selector) + +test('the slot copy control is legible on every theme', async ({ app, page }) => { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-copyside-themes-')) + const left = join(dir, 'alpha.txt') + const right = join(dir, 'beta.txt') + writeFileSync(left, 'one\ntwo\n') + writeFileSync(right, 'one\nTWO\n') + + try { + await stubOpenDialog(app, left) + await page.locator('.slot[data-side="left"] .open').click() + await stubOpenDialog(app, right) + await page.locator('.slot[data-side="right"] .open').click() + await expect(page.locator('.slot[data-side="left"] .name')).toHaveText('alpha.txt') + + const failures = [] + const rows = [] + for (const theme of THEMES) { + await openSettings(page) + const dlg = page.getByRole('dialog', { name: 'Settings' }) + await dlg.getByRole('button', { name: `Use the ${theme.label} theme` }).click() + await page.keyboard.press('Escape') + await dlg.waitFor({ state: 'hidden' }) + await expect(page.locator('html')).toHaveAttribute('data-theme', theme.id) + + // Hovered is the only state it is ever seen in. .btn transitions colour + // over 120ms and getComputedStyle reports the INTERPOLATED value, so + // measuring straight after the hover reads a colour no one ever sees. + await page.locator('.slot[data-side="left"]').hover() + await page.waitForTimeout(250) + const ratio = await contrastOf(page, '.slot[data-side="left"] .copy') + // The file name beside it is --text on the same ground and is already held + // above the reading floor, so it calibrates the instrument: if this ever + // dips, the measurement is wrong before the control is. + const control = await contrastOf(page, '.slot[data-side="left"] .name') + rows.push(`${theme.id.padEnd(10)} copy ${ratio?.toFixed(2)} name ${control?.toFixed(2)}`) + if (!(control >= 4.5)) failures.push(`${theme.id}: INSTRUMENT name=${control?.toFixed(2)}`) + if (!(ratio >= DIM)) failures.push(`${theme.id}: ${ratio?.toFixed(2)}`) + } + + expect(failures, `measured:\n${rows.join('\n')}\n`).toEqual([]) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) diff --git a/e2e/copy-side.spec.mjs b/e2e/copy-side.spec.mjs new file mode 100644 index 00000000..1d3de5a6 --- /dev/null +++ b/e2e/copy-side.spec.mjs @@ -0,0 +1,154 @@ +import { test, expect, clickAppMenuItem, stubOpenDialog } from './fixtures.mjs' +import { zipSync, strToU8 } from 'fflate' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +// Copying one SIDE puts that file's own text on the real OS clipboard, which is +// the half a unit test cannot see: the write goes main-process (clipboard:write, +// since navigator.clipboard is denied here) and the control only exists once the +// slot has laid out. So drive it the way a reader does and read the clipboard +// back through Electron. + +const LEFT_TEXT = 'one\ntwo\nthree\n' +const RIGHT_TEXT = 'one\nTWO\nthree\n' + +function twoFiles() { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-copyside-')) + const left = join(dir, 'alpha.txt') + const right = join(dir, 'beta.txt') + writeFileSync(left, LEFT_TEXT) + writeFileSync(right, RIGHT_TEXT) + return { dir, left, right } +} + +async function loadPair(app, page, left, right) { + await stubOpenDialog(app, left) + await page.locator('.slot[data-side="left"] .open').click() + await stubOpenDialog(app, right) + await page.locator('.slot[data-side="right"] .open').click() + await expect(page.locator('.slot[data-side="right"] .name')).toHaveText('beta.txt') +} + +const readClipboard = (app) => app.evaluate(({ clipboard }) => clipboard.readText()) +const clearClipboard = (app) => app.evaluate(({ clipboard }) => clipboard.clear()) + +const copyButton = (page, side) => page.locator(`.slot[data-side="${side}"] .copy`) + +test('each slot copies its own side verbatim, not the diff between them', async ({ app, page }) => { + const { dir, left, right } = twoFiles() + try { + await loadPair(app, page, left, right) + + await clearClipboard(app) + await copyButton(page, 'left').click() + await expect(page.locator('.notice')).toContainText('alpha.txt') + expect(await readClipboard(app)).toBe(LEFT_TEXT) + + await clearClipboard(app) + await copyButton(page, 'right').click() + expect(await readClipboard(app)).toBe(RIGHT_TEXT) + + // The give-away that this is the side and not Copy Diff as Patch, which + // shares the toolbar and would have written a ---/+++ header. + expect(await readClipboard(app)).not.toContain('---') + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) + +// The Edit menu (and so the palette and Cmd+Shift+1/2, which dispatch the same +// two actions) reaches the same two sides. Driven through the menu item rather +// than a keypress: a CDP-injected key never reaches a native accelerator, so +// asserting on one would prove nothing about the binding. +test('the Edit menu reaches the same two sides', async ({ app, page }) => { + const { dir, left, right } = twoFiles() + try { + await loadPair(app, page, left, right) + + await clearClipboard(app) + await clickAppMenuItem(app, 'Copy Left Side') + await expect.poll(() => readClipboard(app)).toBe(LEFT_TEXT) + + await clearClipboard(app) + await clickAppMenuItem(app, 'Copy Right Side') + await expect.poll(() => readClipboard(app)).toBe(RIGHT_TEXT) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) + +// An opacity-0 button still takes clicks, so the thing that keeps it from +// stealing one meant for the slot is that it sits BESIDE the name, never over +// it. Assert the boxes are disjoint rather than trusting the flex row to stay +// that way. +test('the copy control stays out of the resting row and clear of the name', async ({ + app, + page +}) => { + const { dir, left, right } = twoFiles() + try { + await loadPair(app, page, left, right) + const copy = copyButton(page, 'left') + + await expect(copy).toHaveCSS('opacity', '0') + await page.locator('.slot[data-side="left"]').hover() + await expect(copy).toHaveCSS('opacity', '1') + + const open = await page.locator('.slot[data-side="left"] .open').boundingBox() + const box = await copy.boundingBox() + expect(box.x).toBeGreaterThanOrEqual(open.x + open.width - 1) + + // Icon buttons come off the control scale, never padding + font-size. + const scale = await page.evaluate(() => + getComputedStyle(document.documentElement).getPropertyValue('--control-h-sm').trim() + ) + expect(Math.round(box.height)).toBe(parseInt(scale, 10)) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) + +// A spreadsheet side carries sheets and no text, so there is nothing to put on +// the clipboard — it gets no control rather than one that refuses. +test('a spreadsheet side offers no copy control at all', async ({ app, page }) => { + const XML = '' + const cell = (ref, text) => `${text}` + const book = (value) => + Buffer.from( + zipSync({ + 'xl/workbook.xml': strToU8( + `${XML}` + + '' + ), + 'xl/_rels/workbook.xml.rels': strToU8( + `${XML}' + ), + 'xl/worksheets/sheet1.xml': strToU8( + `${XML}` + + `${cell('A1', 'Region')}${cell('B1', value)}` + ) + }) + ) + + const dir = mkdtempSync(join(tmpdir(), 'diffbro-copyside-xlsx-')) + const left = join(dir, 'a.xlsx') + const right = join(dir, 'b.xlsx') + writeFileSync(left, book('100')) + writeFileSync(right, book('150')) + try { + await stubOpenDialog(app, left) + await page.locator('.slot[data-side="left"] .open').click() + await stubOpenDialog(app, right) + await page.locator('.slot[data-side="right"] .open').click() + await expect(page.locator('.slot[data-side="right"] .name')).toHaveText('b.xlsx') + + await page.locator('.slot[data-side="left"]').hover() + await expect(copyButton(page, 'left')).toHaveCount(0) + await expect(copyButton(page, 'right')).toHaveCount(0) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) diff --git a/e2e/deep-html.spec.mjs b/e2e/deep-html.spec.mjs new file mode 100644 index 00000000..f45b26ce --- /dev/null +++ b/e2e/deep-html.spec.mjs @@ -0,0 +1,42 @@ +import { test, expect, stubOpenDialog } from './fixtures.mjs' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +// Monaco's HTML worker walks the document tree to build symbols with a plain +// recursion and no depth guard (provideFileSymbolsInternal → children.forEach → +// itself). An unclosed tag nests everything after it, so a generated or +// truncated HTML file recurses once per line and overflows the stack — which +// reaches the reader as a crash-report dialog over a file the app should simply +// have shown. Only a real launch runs the worker at all, so this lives here. + +// Two frames per level, against a ~10k frame budget. +const DEPTH = 9000 + +const nested = (marker) => + `\n${'
\n'.repeat(DEPTH)}${marker}\n${'
\n'.repeat(DEPTH)}\n` + +test('a deeply nested HTML file compares without overflowing the worker', async ({ app, page }) => { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-deep-html-')) + const left = join(dir, 'left.html') + const right = join(dir, 'right.html') + writeFileSync(left, nested('ALPHA')) + writeFileSync(right, nested('OMEGA')) + + try { + await stubOpenDialog(app, left) + await page.locator('.slot[data-side="left"] .open').click() + await stubOpenDialog(app, right) + await page.locator('.slot[data-side="right"] .open').click() + + // The diff itself must land: one changed line, deep inside the nesting. + await expect(page.locator('.status-band .del')).toContainText('1 removed', { timeout: 30_000 }) + + // The symbol walk is kicked off after the model settles, so give it the + // chance to blow up before declaring the file safe to open. + await page.waitForTimeout(2000) + await expect(page.getByRole('dialog', { name: 'Something went wrong' })).toHaveCount(0) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) diff --git a/scripts/lib/legacySize.mjs b/scripts/lib/legacySize.mjs index bcf51c6f..893da7fb 100644 --- a/scripts/lib/legacySize.mjs +++ b/scripts/lib/legacySize.mjs @@ -22,9 +22,9 @@ export const LEGACY_SIZE = { 'src/renderer/src/composables/useStreamedDiff.js': { fn: 99 }, 'src/renderer/src/composables/useTabContextMenu.js': { fn: 70 }, 'src/renderer/src/composables/useTagInput.js': { fn: 84 }, - 'src/renderer/src/menus.js': { fn: 87 }, + 'src/renderer/src/menus.js': { fn: 67 }, 'src/renderer/src/monaco-mermaid.js': { fn: 94 }, - 'src/renderer/src/stores/diffStore.js': { file: 747 }, + 'src/renderer/src/stores/diffStore.js': { file: 710 }, 'src/renderer/src/stores/snippetStore.js': { file: 448 }, 'src/renderer/src/stores/tabsStore.js': { file: 323 }, 'src/renderer/src/stores/vaultStore.js': { file: 362 }, diff --git a/src/main/menuSections.js b/src/main/menuSections.js index 20975bb0..81f2abe2 100644 --- a/src/main/menuSections.js +++ b/src/main/menuSections.js @@ -41,6 +41,17 @@ export const editMenu = (send, isMac) => ({ accelerator: 'CmdOrCtrl+Shift+F', click: () => send('copy-diff-file') }, + // Shift over the digit that OPENS that side (CmdOrCtrl+1 / +2). + { + label: t('menu.edit.copyLeft'), + accelerator: 'CmdOrCtrl+Shift+1', + click: () => send('copy-left') + }, + { + label: t('menu.edit.copyRight'), + accelerator: 'CmdOrCtrl+Shift+2', + click: () => send('copy-right') + }, { label: t('menu.edit.applyPatch'), click: () => send('apply-patch') }, { type: 'separator' }, { diff --git a/src/renderer/src/adapters/textAdapter.js b/src/renderer/src/adapters/textAdapter.js index 0216e7ef..82162365 100644 --- a/src/renderer/src/adapters/textAdapter.js +++ b/src/renderer/src/adapters/textAdapter.js @@ -13,6 +13,12 @@ const EXT_TO_LANGUAGE = { tsx: 'typescript', jsx: 'javascript', json: 'json', + // JSON by spec, and all three routinely carry other languages inside their + // string values — a HAR holds whole captured HTML responses. Naming them here + // keeps that payload from deciding what the file is. + har: 'json', + map: 'json', + webmanifest: 'json', css: 'css', scss: 'scss', html: 'html', diff --git a/src/renderer/src/components/FileSlot.vue b/src/renderer/src/components/FileSlot.vue index bb966d4d..9c9442bd 100644 --- a/src/renderer/src/components/FileSlot.vue +++ b/src/renderer/src/components/FileSlot.vue @@ -1,6 +1,10 @@ diff --git a/src/renderer/src/components/FileSlotsRow.vue b/src/renderer/src/components/FileSlotsRow.vue index a473c342..7654f4c7 100644 --- a/src/renderer/src/components/FileSlotsRow.vue +++ b/src/renderer/src/components/FileSlotsRow.vue @@ -16,6 +16,7 @@ const store = useDiffStore() :file="store.left" :awaiting="!store.left && !!store.right" @pick="store.pick('left')" + @copy="store.copySide('left')" />