diff --git a/README.md b/README.md index 696a65c..414e2f7 100644 --- a/README.md +++ b/README.md @@ -44,27 +44,27 @@ Builds are **unsigned**, so SmartScreen and Gatekeeper warn on first launch (the ## What it does -| | | -| ------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Compare** | Two files or pasted text, split or inline, word-level highlights, in-view search, and a live re-diff when a file changes on disk. | -| **Understand structure** | JSON, YAML and XML compared as _data_: reordering keys or reformatting stops counting, and unchanged keys collapse away. | -| **Excel** | `.xlsx` workbooks as aligned grids — sheet tabs and cell-level highlights, with inserted rows and columns that don't cascade into false changes. Dates read as dates, hidden sheets and rows are marked, and formulas are compared as well as their results, so a total pasted over the formula behind it is caught rather than shown as unchanged. Headers are found under a title row rather than assumed to be row 1, and rows can be paired by the columns that name them — one column or several — so the same export sorted differently reads as the one figure that moved instead of as a rewrite. Set a tolerance — one of the presets or a threshold of your own, percentage or raw — and rounding noise stops counting; export the whole change list as a CSV. | -| **CSV** | `.csv` and `.tsv` compare as text or, one toggle away, as the same grid — rows aligned by their first column, quoted fields kept whole. | -| **Dependencies** | A lockfile pair reads as the dependency moves it describes, not as the four thousand lines it is written in: which packages were added, removed, bumped or downgraded, the semver step of each, and — the part that matters — which of them you actually asked for rather than got carried along. `package-lock.json`, `pnpm-lock.yaml`, `yarn.lock`, `go.sum` and `composer.lock`. Nothing is fetched; every fact comes out of the file in front of you. | -| **Huge files** | Past 32 MB a file is indexed by line instead of loaded, and the rows you're looking at are read from disk as you scroll — a multi-gigabyte log opens in seconds. Marked as streamed, with the few actions that need the whole text saying so rather than half-working. | -| **Keep** | Saved diffs: encrypted, tagged, optionally auto-expiring. Drag a row onto another to arrange the list yourself; starred rows stay above the rest. Your open tabs come back on the next launch, and the strip can be told to close the oldest comparison to make room for a new one. | -| **Share** | One signed file only the recipients you ticked can open, carrying the expiry you chose so every copy dies at the same moment. Give a trusted key an email address and Diff Bro opens an addressed message in your own mail app with the sealed file on the clipboard — it never sends anything itself. The key swap rides the same rails: email your key from the My key dialog, and a key copied out of any chat app is offered — fingerprint first — when you press + Trusted key. | -| **Export as image** | A real screenshot of the diff view — your theme, panes and highlighting — cropped to the change and stitched if it's taller than the window. Snippets go the same way, and a Mermaid snippet leaves as its rendered diagram. | -| **Snippets** | An encrypted, tagged text library you can drag straight into the diff pane — two snippets compare like any two files, and editing one updates the comparison on screen. Per-language highlighting, live Mermaid (readable light or dark whatever the app is wearing), Markdown/Jira preview, and secret snippets that render as `****`. Every edit keeps the version it replaced — History in the snippet window lists them by timestamp, each diffed against its predecessor, any of them a copy away. Name one `Standup {{today}}` and the placeholder resolves as you save — `{{now}}`, `{{week}}`, `{{weekday}}` and the rest are listed under the field as you type. Naming is completed inline: type a few characters and the rest of the shared head of your existing names appears ahead of the caret, Tab to take it. Drag a row onto another to arrange the library by hand. | -| **Quick look-up** | A global shortcut searches your snippets and diffs without raising the app; copy one straight to the clipboard, or capture a new one with `Ctrl/Cmd+N` — whatever you searched for becomes its name, and the body is syntax-coloured as you type in whatever language it turns out to be. | -| **Stays out of the way** | On Windows, closing the window keeps Diff Bro in the notification area so the quick look-up shortcut still answers — right-click the icon to exit, and turn either that or start-at-sign-in off in Settings ▸ Desktop. | -| **Language** | Every menu, dialog and label reads from one message catalogue, and Settings ▸ Appearance switches it — menus included, without a restart. English ships today; a new language is a data file, not a code change. | -| **Guided first run** | Six coach marks over the real controls on a first launch — comparing, sealing, the library, then the way into Settings and around it — with four more if you want them. Each step points at a control and its button performs the action, so nothing opens unannounced. Back revisits a step, and everything it put on screen — the demo files, the example snippet — leaves when it does. Escape or Skip ends it for good; Help ▸ Show Tour brings it back. | -| **Diagrams** | Two Mermaid files compare as a picture, not as text — one diagram carrying both revisions, so an inserted node reads as one change instead of a rewrite. | -| **Tools** | JSON, Base64, UUID, JWT, Epoch, URL, Lines, XML, checksums, a regex tester, find & replace, text encryption — rich panels, not blank text boxes. All of them live in their own sidebar section; star the ones you reach for and they stay at the top. | -| **Merge conflicts** | Registered as git's `difftool` **and** `mergetool`: `git mergetool` opens a real three-way view — the two branches either side, the file you are producing in the middle, and that middle one is a full editor. Each side is labelled with the branch it came from and read out of git's index, so no `<<<<<<<` ever reaches the screen; the chevrons in the gutters move a side across, F7 walks the conflicts, and where neither side is right you just type the answer. Thirty conflicted files are thirty stops, counted in the header. It writes the merged file back and tells git it is done — the one file Diff Bro writes over; everything else it produces is a new file you picked the place for. | -| **Terminal** | `diffbro compare a.json b.json` opens a comparison in the running app, and either side can name a git revision instead of a file — `diffbro compare HEAD~1:src/app.js src/app.js` reads the old copy straight out of the repository, so you never have to produce one first. `diffbro open` raises the app, `diffbro backup ` writes an encrypted archive. No port, no daemon. | -| **Yours to arrange** | Twenty themes (Nord, Sepia, Solar, Nyan, Matrix, Volcano, Tide, Graphite, plus accessibility-grade Contrast and Beacon), shared tags, adjustable limits. | +| | | +| ------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Compare** | Two files or pasted text, split or inline, word-level highlights, in-view search, and a live re-diff when a file changes on disk. | +| **Understand structure** | JSON, YAML and XML compared as _data_: reordering keys or reformatting stops counting, and unchanged keys collapse away. | +| **Excel** | `.xlsx` workbooks as aligned grids — sheet tabs and cell-level highlights, with inserted rows and columns that don't cascade into false changes. Dates read as dates, hidden sheets and rows are marked, and formulas are compared as well as their results, so a total pasted over the formula behind it is caught rather than shown as unchanged. Headers are found under a title row rather than assumed to be row 1, and rows can be paired by the columns that name them — one column or several — so the same export sorted differently reads as the one figure that moved instead of as a rewrite. Set a tolerance — one of the presets or a threshold of your own, percentage or raw — and rounding noise stops counting; export the whole change list as a CSV. | +| **CSV** | `.csv` and `.tsv` compare as text or, one toggle away, as the same grid — rows aligned by their first column, quoted fields kept whole. | +| **Dependencies** | A lockfile pair reads as the dependency moves it describes, not as the four thousand lines it is written in: which packages were added, removed, bumped or downgraded, the semver step of each, and — the part that matters — which of them you actually asked for rather than got carried along. `package-lock.json`, `pnpm-lock.yaml`, `yarn.lock`, `go.sum` and `composer.lock`. Nothing is fetched; every fact comes out of the file in front of you. | +| **Huge files** | Past 32 MB a file is indexed by line instead of loaded, and the rows you're looking at are read from disk as you scroll — a multi-gigabyte log opens in seconds. Marked as streamed, with the few actions that need the whole text saying so rather than half-working. | +| **Keep** | Saved diffs: encrypted, tagged, optionally auto-expiring. Drag a row onto another to arrange the list yourself; starred rows stay above the rest. Your open tabs come back on the next launch, and the strip can be told to close the oldest comparison to make room for a new one. | +| **Share** | One signed file only the recipients you ticked can open, carrying the expiry you chose so every copy dies at the same moment. Give a trusted key an email address and Diff Bro opens an addressed message in your own mail app with the sealed file on the clipboard — it never sends anything itself. The key swap rides the same rails: email your key from the My key dialog, and a key copied out of any chat app is offered — fingerprint first — when you press + Trusted key. | +| **Export as image** | A real screenshot of the diff view — your theme, panes and highlighting — cropped to the change and stitched if it's taller than the window. Snippets go the same way, and a Mermaid snippet leaves as its rendered diagram. | +| **Snippets** | An encrypted, tagged text library you can drag straight into the diff pane — two snippets compare like any two files, and editing one updates the comparison on screen. Per-language highlighting, live Mermaid (readable light or dark whatever the app is wearing), Markdown/Jira preview, and secret snippets that render as `****`. Every edit keeps the version it replaced — History in the snippet window lists them by timestamp, each diffed against its predecessor, any of them a copy away. Name one `Standup {{today}}` and the placeholder resolves as you save — `{{now}}`, `{{week}}`, `{{weekday}}` and the rest are listed under the field as you type. Naming is completed inline: type a few characters and the rest of the shared head of your existing names appears ahead of the caret, Tab to take it. Drag a row onto another to arrange the library by hand. | +| **Quick look-up** | A global shortcut searches your snippets and diffs without raising the app; copy one straight to the clipboard, or capture a new one with `Ctrl/Cmd+N` — whatever you searched for becomes its name, and the body is syntax-coloured as you type in whatever language it turns out to be. | +| **Stays out of the way** | On Windows, closing the window keeps Diff Bro in the notification area so the quick look-up shortcut still answers — right-click the icon to exit, and turn either that or start-at-sign-in off in Settings ▸ Desktop. | +| **Language** | Every menu, dialog and label reads from one message catalogue, and Settings ▸ Appearance switches it — menus included, without a restart. English ships today; a new language is a data file, not a code change. | +| **Guided first run** | Six coach marks over the real controls on a first launch — comparing, sealing, the library, then the way into Settings and around it — with four more if you want them. Each step points at a control and its button performs the action, so nothing opens unannounced. Back revisits a step, and everything it put on screen — the demo files, the example snippet — leaves when it does. Escape or Skip ends it for good; Help ▸ Show Tour brings it back. | +| **Diagrams** | Two Mermaid files compare as a picture, not as text — one diagram carrying both revisions, so an inserted node reads as one change instead of a rewrite. | +| **Tools** | JSON, Base64, UUID, JWT, Epoch, URL, Lines, XML, checksums, a regex tester, find & replace, text encryption — rich panels, not blank text boxes. All of them live in their own sidebar section; star the ones you reach for and they stay at the top. | +| **Merge conflicts** | Registered as git's `difftool` **and** `mergetool`: `git mergetool` opens the list of every conflicted file first — what is left, what is done, and how many regions each still holds. Take a whole side from a row without opening anything, or pick one for a real three-way view: the two branches either side, the file you are producing in the middle, and that middle one is a full editor. Each side is labelled with the branch it came from and read out of git's index, so no `<<<<<<<` ever reaches the screen; a button on each pane's inner edge moves that side across, F7 walks the conflicts, and where neither side is right you just type the answer. Resolve them in any order — saving returns you to the list, and closing it never ends the merge. It writes each merged file back and tells git it is done — the only files Diff Bro writes over; everything else it produces is a new file you picked the place for. | +| **Terminal** | `diffbro compare a.json b.json` opens a comparison in the running app, and either side can name a git revision instead of a file — `diffbro compare HEAD~1:src/app.js src/app.js` reads the old copy straight out of the repository, so you never have to produce one first. `diffbro open` raises the app, `diffbro backup ` writes an encrypted archive. No port, no daemon. | +| **Yours to arrange** | Twenty themes (Nord, Sepia, Solar, Nyan, Matrix, Volcano, Tide, Graphite, plus accessibility-grade Contrast and Beacon), shared tags, adjustable limits. |
The smaller things diff --git a/docs/brand/roadmap.svg b/docs/brand/roadmap.svg index a185c5a..ca6054c 100644 --- a/docs/brand/roadmap.svg +++ b/docs/brand/roadmap.svg @@ -5,7 +5,7 @@ sidebar. Track hues are real theme accents from src/renderer/src/utils/themes.js (Dim, Bloom, Neon, Beacon) — colour depth encodes sequence: solid now, faded later. Keep this in step with the items in docs/roadmap.md. --> - + @@ -117,7 +117,7 @@ Compare a git revision - Three-way merge view + Merge: conflicts list + three-way utils/lockfile/ diff --git a/docs/ipc-security.md b/docs/ipc-security.md index 0d2c515..4eb8ef2 100644 --- a/docs/ipc-security.md +++ b/docs/ipc-security.md @@ -69,29 +69,30 @@ There is no other door. These are the non-negotiables from [standards.md](standards.md), and the file that enforces each: -| Guard | What it stops | Where | -| ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------ | -| **Sandbox + `contextIsolation`** | Renderer has no Node/Electron globals; can't `require('fs')` | `window.js` (webPreferences) | -| **Network kill switch** | `webRequest.onBeforeRequest` cancels every request that isn't `file:`/`blob:`/`data:` (fetch, XHR, images, workers…) | `security.js` | -| **CSP** | `connect-src 'self'`, `object-src 'none'` — second layer against outbound requests | renderer CSP meta | -| **Deny-all permission handler** | `setPermissionRequestHandler` rejects camera, clipboard-read, geolocation, everything | `security.js` | -| **`will-navigate` / `setWindowOpenHandler`** | Renderer can't navigate away or open external windows | `security.js` | -| **Path provenance allowlist** | `file:read` only serves a path the user actually picked or dropped — not one the renderer invents | `files.js` | -| **A store name is a key, never a path** | `store:load` / `store:save` turn a renderer-supplied name into `/.json`, so a traversing one escaped the folder and `trusted-keys` reached the trust store through a key/value channel. The name must be one of `STORE_NAMES` — the closed set the stores actually use — and anything else is REFUSED, not sanitised into an adjacent file | `appData.js`, `dataFiles.js` | -| **Streamed window bounds** | `stream:lines` serves only a session token main issued, for a side and a line range that is validated and **refused** (never clamped) when out of range or wider than the row ceiling; the paths behind a session clear the same `mayReadPath` gate as any other read | `streamedDiff.js`, `streamWindow.js` | -| **Keys never cross IPC** | Vault/identity keys stay behind `safeStorage`; only ciphertext is ever returned | `vault.js`, `vaultCrypt.js` | -| **Untrusted-input caps** | Import files get size caps, shape validation, recomputed fingerprints; `.xlsx` gets decompression-bomb caps, a cell budget, a per-cell formula-length cap and a ceiling on how many cells may carry extras | `files.js`, `xlsx/*`, `share.js` | -| **Export format allowlist** | `diff:exportFile` is the ONE place the app writes renderer-supplied text. The extension comes from a fixed table in main keyed by a `format` name, never from the renderer's own string, so no caller can ask for an executable one; the change register additionally defuses fields a spreadsheet would run as formulas (`=`, `+`, `-`, `@`) | `files.js`, `changeRegister.js` | -| **Restored bundles are vetted in main** | A decryptable `.diffbroconf` is still a file off disk. `validateRestoredConfig` caps the trust list, the snippet bundle (`SNIPPET_LIMITS`) **and the saved-diff bundle** (`VAULT_LIMITS` — 5000 diffs, 512-byte names, 20 tags each) before the renderer is handed anything to re-encrypt, so a crafted backup cannot drive an unbounded loop of `vault:encrypt` calls | `shareCore.js`, `snippetSealing.js` | -| **A CLI-named write path is validated in main** | `config:backupTo` takes the destination `diffbro backup ` supplied — a string typed into a shell. `backupZip.checkDestination` refuses a directory, a missing parent, an existing file, and anything resolving inside the app's own data directory, before a byte is written; the renderer never picks the path and never sees the sealed blob | `share.js`, `backupZip.js` | -| **Backup deletion by age, never by name** | `backup:prune` is the only handler that DELETES. It takes an age in days that must be one of the two the app offers (`PRUNE_DAYS`), never a path or a filename, so the renderer cannot name a file to remove; every candidate comes from `listBackups`, which yields only names that parse as one of ours, so anything else sharing the folder is untouched | `backupRoute.js`, `autoBackup.js` | -| **The mail hand-off supplies no URL and no path** | `mail:handoff` takes fingerprints and text. Main resolves the addresses from the trust store, BUILDS the `mailto:` (`mailto.js`), and re-checks it with `isSafeMailtoUrl` before `shell.openExternal` — `mailto:` only, and an `attach`/`attachment` parameter is refused rather than ignored. The file it copies and reveals is the path it just sealed, never one round-tripped through the renderer | `mail.js`, `mailto.js`, `linkPolicy.js`, `mailAddress.js` | -| **Copy as file takes bytes, never a path** | `clipboard:writeFile` receives content and a DISPLAY NAME. Main slugs the name flat (so `../../.ssh/config` cannot traverse), stages it in a `0o700` directory, and puts that path on the clipboard. The renderer cannot name a file to stage, read one back, or learn the staging directory; staged copies are pruned at 30 minutes and swept on quit **and** on next launch | `clipboardCopy.js`, `clipboardStage.js`, `clipboardWrite.js` | -| **The merge write takes TEXT, never a path** | `merge:write` is the only handler that writes over a file the user already had, and it can only write the `$MERGED` path main was launched with by `git mergetool`. The renderer sends the resolved text; there is no argument for a filename. With no merge launch in progress the handler writes nothing at all, and one launch permits one write | `mergeSession.js`, `cliRoute.js` | -| **The tray settings are booleans** | `tray:supported`, `app:startAtLogin` and `app:setStartAtLogin` take and return nothing but booleans. The login item registers `process.execPath` — main's own — with a fixed `--hidden` argument; the renderer never supplies an executable, an argument or a registry key, and there is no handler that would accept one | `tray.js`, `trayCore.js` | -| **A stored address cannot become a header** | `share:setTrustedEmail` refuses anything carrying CR/LF, a comma, a semicolon, angle brackets or whitespace, **before it reaches disk** — otherwise a stored address would inject a second header into the hand-off URL. A restored backup's `email` field is dropped if it fails the same check | `trustedKeys.js`, `mailAddress.js`, `shareCore.js` | -| **No injection sinks** | `v-html`, `eval`, `new Function`, `innerHTML` are ESLint-banned | `eslint.config.mjs` | -| **Capture rect clamped** | `image:capture` / `image:appendSlice` screenshot only a region clamped inside the window's own content, never a forged or unbounded one; a stitched export is capped in height so a renderer-driven loop can't exhaust memory, and the bitmap stays in main | `captureRect.js`, `stitchBitmap.js`, `diffImage.js` | +| Guard | What it stops | Where | +| ------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------ | +| **Sandbox + `contextIsolation`** | Renderer has no Node/Electron globals; can't `require('fs')` | `window.js` (webPreferences) | +| **Network kill switch** | `webRequest.onBeforeRequest` cancels every request that isn't `file:`/`blob:`/`data:` (fetch, XHR, images, workers…) | `security.js` | +| **CSP** | `connect-src 'self'`, `object-src 'none'` — second layer against outbound requests | renderer CSP meta | +| **Deny-all permission handler** | `setPermissionRequestHandler` rejects camera, clipboard-read, geolocation, everything | `security.js` | +| **`will-navigate` / `setWindowOpenHandler`** | Renderer can't navigate away or open external windows | `security.js` | +| **Path provenance allowlist** | `file:read` only serves a path the user actually picked or dropped — not one the renderer invents | `files.js` | +| **A store name is a key, never a path** | `store:load` / `store:save` turn a renderer-supplied name into `/.json`, so a traversing one escaped the folder and `trusted-keys` reached the trust store through a key/value channel. The name must be one of `STORE_NAMES` — the closed set the stores actually use — and anything else is REFUSED, not sanitised into an adjacent file | `appData.js`, `dataFiles.js` | +| **Streamed window bounds** | `stream:lines` serves only a session token main issued, for a side and a line range that is validated and **refused** (never clamped) when out of range or wider than the row ceiling; the paths behind a session clear the same `mayReadPath` gate as any other read | `streamedDiff.js`, `streamWindow.js` | +| **Keys never cross IPC** | Vault/identity keys stay behind `safeStorage`; only ciphertext is ever returned | `vault.js`, `vaultCrypt.js` | +| **Untrusted-input caps** | Import files get size caps, shape validation, recomputed fingerprints; `.xlsx` gets decompression-bomb caps, a cell budget, a per-cell formula-length cap and a ceiling on how many cells may carry extras | `files.js`, `xlsx/*`, `share.js` | +| **Export format allowlist** | `diff:exportFile` is the ONE place the app writes renderer-supplied text. The extension comes from a fixed table in main keyed by a `format` name, never from the renderer's own string, so no caller can ask for an executable one; the change register additionally defuses fields a spreadsheet would run as formulas (`=`, `+`, `-`, `@`) | `files.js`, `changeRegister.js` | +| **Restored bundles are vetted in main** | A decryptable `.diffbroconf` is still a file off disk. `validateRestoredConfig` caps the trust list, the snippet bundle (`SNIPPET_LIMITS`) **and the saved-diff bundle** (`VAULT_LIMITS` — 5000 diffs, 512-byte names, 20 tags each) before the renderer is handed anything to re-encrypt, so a crafted backup cannot drive an unbounded loop of `vault:encrypt` calls | `shareCore.js`, `snippetSealing.js` | +| **A CLI-named write path is validated in main** | `config:backupTo` takes the destination `diffbro backup ` supplied — a string typed into a shell. `backupZip.checkDestination` refuses a directory, a missing parent, an existing file, and anything resolving inside the app's own data directory, before a byte is written; the renderer never picks the path and never sees the sealed blob | `share.js`, `backupZip.js` | +| **Backup deletion by age, never by name** | `backup:prune` is the only handler that DELETES. It takes an age in days that must be one of the two the app offers (`PRUNE_DAYS`), never a path or a filename, so the renderer cannot name a file to remove; every candidate comes from `listBackups`, which yields only names that parse as one of ours, so anything else sharing the folder is untouched | `backupRoute.js`, `autoBackup.js` | +| **The mail hand-off supplies no URL and no path** | `mail:handoff` takes fingerprints and text. Main resolves the addresses from the trust store, BUILDS the `mailto:` (`mailto.js`), and re-checks it with `isSafeMailtoUrl` before `shell.openExternal` — `mailto:` only, and an `attach`/`attachment` parameter is refused rather than ignored. The file it copies and reveals is the path it just sealed, never one round-tripped through the renderer | `mail.js`, `mailto.js`, `linkPolicy.js`, `mailAddress.js` | +| **Copy as file takes bytes, never a path** | `clipboard:writeFile` receives content and a DISPLAY NAME. Main slugs the name flat (so `../../.ssh/config` cannot traverse), stages it in a `0o700` directory, and puts that path on the clipboard. The renderer cannot name a file to stage, read one back, or learn the staging directory; staged copies are pruned at 30 minutes and swept on quit **and** on next launch | `clipboardCopy.js`, `clipboardStage.js`, `clipboardWrite.js` | +| **The merge write takes TEXT, never a path** | `merge:write` writes over a file the user already had, and can only write the row the view has open — the `$MERGED` path main was launched with, or a row of the conflicts list main built. The renderer sends the resolved text; there is no argument for a filename. With no merge launch in progress the handler writes nothing at all, and one launch permits one write | `mergeSession.js`, `mergeRoute.js` | +| **A conflicted file is named by INDEX** | `merge:take` and `merge:open` address a row by its position in the list main enumerated, never by path — a row carries only a display name and a directory label. Main recomputes the path from the repository root, and re-verifies it against `git diff --diff-filter=U` at WRITE time, so a row that stopped being unmerged since the dialog opened fails closed. `merge:list` returns rows; `merge:endWalk` forgets them | `conflictSession.js`, `conflictList.js`, `mergeRoute.js` | +| **The tray settings are booleans** | `tray:supported`, `app:startAtLogin` and `app:setStartAtLogin` take and return nothing but booleans. The login item registers `process.execPath` — main's own — with a fixed `--hidden` argument; the renderer never supplies an executable, an argument or a registry key, and there is no handler that would accept one | `tray.js`, `trayCore.js` | +| **A stored address cannot become a header** | `share:setTrustedEmail` refuses anything carrying CR/LF, a comma, a semicolon, angle brackets or whitespace, **before it reaches disk** — otherwise a stored address would inject a second header into the hand-off URL. A restored backup's `email` field is dropped if it fails the same check | `trustedKeys.js`, `mailAddress.js`, `shareCore.js` | +| **No injection sinks** | `v-html`, `eval`, `new Function`, `innerHTML` are ESLint-banned | `eslint.config.mjs` | +| **Capture rect clamped** | `image:capture` / `image:appendSlice` screenshot only a region clamped inside the window's own content, never a forged or unbounded one; a stitched export is capped in height so a renderer-driven loop can't exhaust memory, and the bitmap stays in main | `captureRect.js`, `stitchBitmap.js`, `diffImage.js` | **One surface worth naming.** Electron cannot put a shell-paste-able file on the Windows clipboard: `clipboard.writeBuffer` is not additive (each call REPLACES the diff --git a/docs/roadmap.md b/docs/roadmap.md index c0d5023..882febf 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -194,9 +194,10 @@ flowchart LR end subgraph merge["three-way merge"] m["index :1: :2: :3: — no markers on screen"] + f["conflicts list — every file, any order"] v["MergeView — ours │ result │ theirs, middle editable"] - w["mergeSession.js — the one write"] - m --> v --> w + w["conflictSession.js — every write verified"] + m --> f --> v --> w end ``` @@ -205,14 +206,17 @@ flowchart LR - **Revisions** — `diffbro compare HEAD~1:src/app.js src/app.js`. `git show` behind a fence: fixed argv, no shell, the repo root computed in main, hooks and the fsmonitor disabled, every inherited `GIT_*` dropped -- **Merge** — `git mergetool` now finishes, in a three-pane view: the two - branches either side, named by branch, and the file you are producing in the - middle as a real editor. Sides come from the index, so no `<<<<<<<` reaches - the screen; gutter chevrons move a side across, F7 walks the conflicts, and - typing IS the answer where neither side was right. This CROSSES "Diff Bro - never writes files", deliberately: the app had already registered for the job. - Main writes only the `$MERGED` path it was launched with, the renderer sends - text and never a path, and the launcher waits so `trustExitCode` is honest +- **Merge** — `git mergetool` now finishes, and opens on the LIST: every + conflicted file, what is left and what is done. Take a whole side from a row, + or open one in the three-pane view — the two branches either side, named by + branch, and the file you are producing in the middle as a real editor. Sides + come from the index, so no `<<<<<<<` reaches the screen; a button on each + pane's inner edge moves that side across, F7 walks the conflicts, and typing + IS the answer where neither side was right. Files are answered in any order. + This CROSSES "Diff Bro never writes files", deliberately: the app had already + registered for the job. The renderer names a row by INDEX and never a path, + every write is re-verified against git's unmerged list on the way in, and the + launcher waits so `trustExitCode` is honest **Open.** TOML lockfiles (`Cargo.lock`, `poetry.lock`) need a parser this repo does not have. A revision PICKER — the app takes a revision, it is not a git diff --git a/docs/screenshots/themes/conflict-list-amber.png b/docs/screenshots/themes/conflict-list-amber.png new file mode 100644 index 0000000..1de66be Binary files /dev/null and b/docs/screenshots/themes/conflict-list-amber.png differ diff --git a/docs/screenshots/themes/conflict-list-beacon.png b/docs/screenshots/themes/conflict-list-beacon.png new file mode 100644 index 0000000..712f374 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-beacon.png differ diff --git a/docs/screenshots/themes/conflict-list-bloom.png b/docs/screenshots/themes/conflict-list-bloom.png new file mode 100644 index 0000000..5d8c004 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-bloom.png differ diff --git a/docs/screenshots/themes/conflict-list-contrast.png b/docs/screenshots/themes/conflict-list-contrast.png new file mode 100644 index 0000000..24ffcaa Binary files /dev/null and b/docs/screenshots/themes/conflict-list-contrast.png differ diff --git a/docs/screenshots/themes/conflict-list-dark.png b/docs/screenshots/themes/conflict-list-dark.png new file mode 100644 index 0000000..3407715 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-dark.png differ diff --git a/docs/screenshots/themes/conflict-list-dim.png b/docs/screenshots/themes/conflict-list-dim.png new file mode 100644 index 0000000..b03fe4b Binary files /dev/null and b/docs/screenshots/themes/conflict-list-dim.png differ diff --git a/docs/screenshots/themes/conflict-list-ember.png b/docs/screenshots/themes/conflict-list-ember.png new file mode 100644 index 0000000..084c744 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-ember.png differ diff --git a/docs/screenshots/themes/conflict-list-graphite.png b/docs/screenshots/themes/conflict-list-graphite.png new file mode 100644 index 0000000..288faa8 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-graphite.png differ diff --git a/docs/screenshots/themes/conflict-list-light.png b/docs/screenshots/themes/conflict-list-light.png new file mode 100644 index 0000000..9d10f1f Binary files /dev/null and b/docs/screenshots/themes/conflict-list-light.png differ diff --git a/docs/screenshots/themes/conflict-list-linen.png b/docs/screenshots/themes/conflict-list-linen.png new file mode 100644 index 0000000..5c60bef Binary files /dev/null and b/docs/screenshots/themes/conflict-list-linen.png differ diff --git a/docs/screenshots/themes/conflict-list-matrix.png b/docs/screenshots/themes/conflict-list-matrix.png new file mode 100644 index 0000000..672e538 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-matrix.png differ diff --git a/docs/screenshots/themes/conflict-list-meridian.png b/docs/screenshots/themes/conflict-list-meridian.png new file mode 100644 index 0000000..70c698f Binary files /dev/null and b/docs/screenshots/themes/conflict-list-meridian.png differ diff --git a/docs/screenshots/themes/conflict-list-neon.png b/docs/screenshots/themes/conflict-list-neon.png new file mode 100644 index 0000000..4cf4502 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-neon.png differ diff --git a/docs/screenshots/themes/conflict-list-nord.png b/docs/screenshots/themes/conflict-list-nord.png new file mode 100644 index 0000000..6f5ff34 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-nord.png differ diff --git a/docs/screenshots/themes/conflict-list-nyan.png b/docs/screenshots/themes/conflict-list-nyan.png new file mode 100644 index 0000000..fc2c55b Binary files /dev/null and b/docs/screenshots/themes/conflict-list-nyan.png differ diff --git a/docs/screenshots/themes/conflict-list-sepia.png b/docs/screenshots/themes/conflict-list-sepia.png new file mode 100644 index 0000000..899cfe8 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-sepia.png differ diff --git a/docs/screenshots/themes/conflict-list-solar.png b/docs/screenshots/themes/conflict-list-solar.png new file mode 100644 index 0000000..c25f6b9 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-solar.png differ diff --git a/docs/screenshots/themes/conflict-list-tide.png b/docs/screenshots/themes/conflict-list-tide.png new file mode 100644 index 0000000..67ec5b7 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-tide.png differ diff --git a/docs/screenshots/themes/conflict-list-vector.png b/docs/screenshots/themes/conflict-list-vector.png new file mode 100644 index 0000000..3f79a26 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-vector.png differ diff --git a/docs/screenshots/themes/conflict-list-volcano.png b/docs/screenshots/themes/conflict-list-volcano.png new file mode 100644 index 0000000..f60a9d4 Binary files /dev/null and b/docs/screenshots/themes/conflict-list-volcano.png differ diff --git a/docs/security.md b/docs/security.md index 40542b6..21aa226 100644 --- a/docs/security.md +++ b/docs/security.md @@ -64,6 +64,32 @@ takes bytes and a display name. With no mergetool launch there is no path held, so the handler writes nothing at all whatever arrives, and a session is spent once used. +### The conflicts list widens that to the whole walk + +`git mergetool` walks the conflicted files one launch at a time, and the list +lets a reader answer them in their own order — so main writes files this +launch's argv did not name. That is a real widening of the most sensitive +surface in the app, and it is fenced the same way rather than differently: + +- The renderer addresses a row by its **index** into the list main built + (`merge:take`, `merge:open`). It never sees a path — a row carries a display + name and a directory label — and there is no handler that takes one. +- Every path is recomputed in main from the repository root plus that row's + entry, and `isRepoRelative` refuses anything absolute, option-shaped, or + climbing out with `..`. +- Verification happens at **use** time, never at list time. A row can stop being + unmerged between the dialog opening and the click, so each write re-runs + `git diff --name-only --diff-filter=U` and refuses a row that is no longer in + it. A stale list fails closed. +- A file answered out of order still gets a launch of its own later. It no + longer has markers, so it is short-circuited **before** the marker guard and + released with a `written` sentinel; refused there instead, it would write no + sentinel and hang the terminal for the launcher's full two hours. +- The list itself is built from a size-capped, binary-sniffed read of each + working copy. A file that cannot be opened as text is listed as `blocked` and + can only be answered whole-file from the index — never decoded and written + back. + The middle pane is a full editor, and that costs nothing here: `merge:write` already took arbitrary text, so a renderer that wanted to write something neither side said could always do it. Editing widens what the READER can diff --git a/e2e/merge-conflicts-list.spec.mjs b/e2e/merge-conflicts-list.spec.mjs new file mode 100644 index 0000000..519bb37 --- /dev/null +++ b/e2e/merge-conflicts-list.spec.mjs @@ -0,0 +1,322 @@ +import { test, expect, launchApp, freshUserDataDir, firstReadyPage } from './fixtures.mjs' +import { workerEnv } from './workerEnv.mjs' +import { execFileSync, spawn } from 'node:child_process' +import { existsSync, mkdtempSync, readFileSync, writeFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { createRequire } from 'node:module' +import { fileURLToPath } from 'node:url' + +const ROOT = fileURLToPath(new URL('..', import.meta.url)) +const MAIN = join(ROOT, 'build', 'main', 'index.js') +const ELECTRON = createRequire(import.meta.url)('electron') + +// Three conflicted files in one merge — the state `git mergetool` walks and the +// only thing that proves the list is real. jsdom cannot see any of it: the +// enumeration is a git call in main, and the walk spans separate launches. +const NAMES = ['alpha.txt', 'beta.txt', 'gamma.txt'] + +function conflictedRepo() { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-walk-')) + const env = { + ...process.env, + GIT_AUTHOR_NAME: 'T', + GIT_AUTHOR_EMAIL: 't@e', + GIT_COMMITTER_NAME: 'T', + GIT_COMMITTER_EMAIL: 't@e' + } + const git = (...args) => execFileSync('git', args, { cwd: dir, env }) + const write = (name, body) => writeFileSync(join(dir, name), body) + git('init', '-q', '-b', 'main') + for (const name of NAMES) write(name, 'one\nbase\nthree\n') + git('add', '.') + git('commit', '-qm', 'base') + git('checkout', '-qb', 'feature') + for (const name of NAMES) write(name, 'one\ntheirs\nthree\n') + git('commit', '-qam', 'theirs') + git('checkout', '-q', 'main') + for (const name of NAMES) write(name, 'one\nours\nthree\n') + git('commit', '-qam', 'ours') + try { + git('merge', 'feature') + } catch { + // Expected: this is the conflict under test. + } + return { dir, path: (name) => join(dir, name) } +} + +function runMergetool(userDataDir, dir, file) { + const env = { ...workerEnv(userDataDir) } + delete env.ELECTRON_RUN_AS_NODE + return new Promise((resolve) => { + const p = spawn( + ELECTRON, + [MAIN, `--user-data-dir=${userDataDir}`, 'mergetool', file, file, file], + { cwd: dir, env, stdio: 'ignore' } + ) + p.on('exit', () => resolve()) + setTimeout(resolve, 8000) + }) +} + +// The launcher polls for this beside $MERGED and exits on it; `written` is what +// tells git the file was resolved. +const sentinel = (file) => `${file}.diffbro-merge-done` + +test('opens the list of every conflicted file before the merge', async () => { + const repo = conflictedRepo() + const userDataDir = freshUserDataDir() + const app = await launchApp(userDataDir) + const page = await firstReadyPage(app) + try { + await runMergetool(userDataDir, repo.dir, repo.path('alpha.txt')) + + const dialog = page.getByTestId('conflicts-dialog') + await expect(dialog).toBeVisible({ timeout: 20000 }) + for (const name of NAMES) { + await expect(page.getByTestId(`conflict-row-${name}`)).toBeVisible() + } + // The three-way view is behind it, on the file git actually asked about. + await expect(page.locator('.merge-view')).toBeVisible() + } finally { + await app.close().catch(() => {}) + rmSync(repo.dir, { recursive: true, force: true }) + } +}) + +// The whole point of the list: git asked about alpha, and the reader answers +// gamma. Main computes that path from the repo root plus the row's index — the +// renderer never names a file. +test('answers a file out of git’s order, from its row', async () => { + const repo = conflictedRepo() + const userDataDir = freshUserDataDir() + const app = await launchApp(userDataDir) + const page = await firstReadyPage(app) + try { + await runMergetool(userDataDir, repo.dir, repo.path('alpha.txt')) + await expect(page.getByTestId('conflicts-dialog')).toBeVisible({ timeout: 20000 }) + + // The row's answers are quiet until the row is pointed at, the way they are + // for a reader. + await page.getByTestId('conflict-row-gamma.txt').hover() + await page.getByTestId('conflict-theirs-gamma.txt').click() + + // Written from the index, with no editor opened for it. + await expect + .poll(() => readFileSync(repo.path('gamma.txt'), 'utf8'), { timeout: 10000 }) + .toBe('one\ntheirs\nthree\n') + // alpha is the file git asked about and is untouched. + expect(readFileSync(repo.path('alpha.txt'), 'utf8')).toContain('<<<<<<<') + + const row = page.getByTestId('conflict-row-gamma.txt') + await expect(row).toHaveClass(/done/) + } finally { + await app.close().catch(() => {}) + rmSync(repo.dir, { recursive: true, force: true }) + } +}) + +// A file answered out of order still gets a launch of its own later. Left to the +// marker guard it would be refused with NO sentinel written, and the terminal +// running `git mergetool` would wait out the launcher's two hours. +test('a relaunch for an already-answered file releases git without a window', async () => { + const repo = conflictedRepo() + const userDataDir = freshUserDataDir() + const app = await launchApp(userDataDir) + const page = await firstReadyPage(app) + try { + await runMergetool(userDataDir, repo.dir, repo.path('alpha.txt')) + await expect(page.getByTestId('conflicts-dialog')).toBeVisible({ timeout: 20000 }) + await page.getByTestId('conflict-row-gamma.txt').hover() + await page.getByTestId('conflict-theirs-gamma.txt').click() + await expect(page.getByTestId('conflict-row-gamma.txt')).toHaveClass(/done/, { timeout: 10000 }) + + const gamma = repo.path('gamma.txt') + rmSync(sentinel(gamma), { force: true }) + await runMergetool(userDataDir, repo.dir, gamma) + + await expect.poll(() => existsSync(sentinel(gamma)), { timeout: 10000 }).toBe(true) + expect(readFileSync(sentinel(gamma), 'utf8')).toBe('written') + // Still showing alpha: the relaunch never became a view of its own. + await expect(page.getByTestId('merge-walk')).toContainText('1 of 3') + } finally { + await app.close().catch(() => {}) + rmSync(repo.dir, { recursive: true, force: true }) + } +}) + +// The defect the design review found: dismissing the list used to be the end of +// it. Closing it must leave the walk alive and offer a way back. +test('closing the list keeps the merge alive, and the walk chip reopens it', async () => { + const repo = conflictedRepo() + const userDataDir = freshUserDataDir() + const app = await launchApp(userDataDir) + const page = await firstReadyPage(app) + try { + await runMergetool(userDataDir, repo.dir, repo.path('alpha.txt')) + const dialog = page.getByTestId('conflicts-dialog') + await expect(dialog).toBeVisible({ timeout: 20000 }) + + await page.keyboard.press('Escape') + await expect(dialog).toHaveCount(0) + // The merge is still under way — Escape closed a list, not a session. + await expect(page.locator('.merge-view')).toBeVisible() + + await page.getByTestId('merge-walk').click() + await expect(dialog).toBeVisible() + + await page.getByTestId('conflicts-close').click() + await expect(dialog).toHaveCount(0) + await expect(page.getByTestId('toolbar-conflicts')).toBeVisible() + await page.getByTestId('toolbar-conflicts').click() + await expect(dialog).toBeVisible() + } finally { + await app.close().catch(() => {}) + rmSync(repo.dir, { recursive: true, force: true }) + } +}) + +// Opening a row that is not the launch's own file and saving it: the write goes +// to that row, verified against git's unmerged list on the way in. +test('opens another row in the three-way view and saves that file', async () => { + const repo = conflictedRepo() + const userDataDir = freshUserDataDir() + const app = await launchApp(userDataDir) + const page = await firstReadyPage(app) + try { + await runMergetool(userDataDir, repo.dir, repo.path('alpha.txt')) + await expect(page.getByTestId('conflicts-dialog')).toBeVisible({ timeout: 20000 }) + + await page.getByTestId('conflict-row-beta.txt').click() + await expect(page.getByTestId('conflicts-dialog')).toHaveCount(0) + await expect(page.locator('.merge-view')).toBeVisible() + await expect(page.locator('.merge-file')).toHaveText('beta.txt') + + await page.getByTestId('merge-take-ours').click() + const save = page.getByTestId('merge-save') + await expect(save).toBeEnabled() + await save.click() + + await expect + .poll(() => readFileSync(repo.path('beta.txt'), 'utf8'), { timeout: 10000 }) + .toBe('one\nours\nthree\n') + // Saving returns the reader to the list, where the remaining work is. + await expect(page.getByTestId('conflicts-dialog')).toBeVisible({ timeout: 10000 }) + } finally { + await app.close().catch(() => {}) + rmSync(repo.dir, { recursive: true, force: true }) + } +}) + +// The take control moved out of Monaco's glyph margin onto the pane's inner +// edge, because a glyph margin only exists on an editor's LEFT edge and is not +// reachable from the keyboard. Only a real editor places these. +test('the take buttons sit on the panes’ inner edges, against the result', async () => { + const repo = conflictedRepo() + const userDataDir = freshUserDataDir() + const app = await launchApp(userDataDir) + const page = await firstReadyPage(app) + try { + await runMergetool(userDataDir, repo.dir, repo.path('alpha.txt')) + await expect(page.getByTestId('conflicts-dialog')).toBeVisible({ timeout: 20000 }) + await page.keyboard.press('Escape') + + const ours = page.getByTestId('merge-take-ours-0') + const theirs = page.getByTestId('merge-take-theirs-0') + await expect(ours).toBeVisible() + await expect(theirs).toBeVisible() + + const panes = page.locator('.merge-pane') + const [oursPane, resultPane, theirsPane] = await Promise.all( + [0, 1, 2].map((i) => panes.nth(i).boundingBox()) + ) + const oursBox = await ours.boundingBox() + const theirsBox = await theirs.boundingBox() + + // Ours straddles the divider between the left pane and the result… + expect(Math.abs(oursBox.x + oursBox.width / 2 - oursPane.x - oursPane.width)).toBeLessThan(4) + // …and theirs the one between the result and the right pane. + expect(Math.abs(theirsBox.x + theirsBox.width / 2 - theirsPane.x)).toBeLessThan(4) + // Neither is out at the window's edge, which is where the glyph margin put + // the ours chevron. + expect(oursBox.x).toBeGreaterThan(resultPane.x - 40) + + // A real button, so it answers a keyboard too. + await theirs.focus() + await page.keyboard.press('Enter') + await expect(page.getByTestId('merge-save')).toBeEnabled() + } finally { + await app.close().catch(() => {}) + rmSync(repo.dir, { recursive: true, force: true }) + } +}) + +// The most likely path through the new dialog: the launch's OWN file is +// pre-selected, so the primary Resolve button opens it. Saving used to write the +// file and never release the launcher — the terminal running `git mergetool` +// then waited out its full two hours on a decision already made. +test('resolving the launch’s own file from the list still releases git', async () => { + const repo = conflictedRepo() + const userDataDir = freshUserDataDir() + const app = await launchApp(userDataDir) + const page = await firstReadyPage(app) + const alpha = repo.path('alpha.txt') + try { + rmSync(sentinel(alpha), { force: true }) + await runMergetool(userDataDir, repo.dir, alpha) + await expect(page.getByTestId('conflicts-dialog')).toBeVisible({ timeout: 20000 }) + + // The pre-selected row IS alpha, so this is the button a reader reaches for. + await page.getByTestId('conflicts-resolve').click() + await expect(page.locator('.merge-file')).toHaveText('alpha.txt') + await page.getByTestId('merge-take-ours').click() + await page.getByTestId('merge-save').click() + + await expect.poll(() => existsSync(sentinel(alpha)), { timeout: 10000 }).toBe(true) + expect(readFileSync(sentinel(alpha), 'utf8')).toBe('written') + expect(readFileSync(alpha, 'utf8')).toBe('one\nours\nthree\n') + } finally { + await app.close().catch(() => {}) + rmSync(repo.dir, { recursive: true, force: true }) + } +}) + +// A launch for a file in NO repository used to leave the previous repository's +// row armed, so the next Save wrote THAT repo's file with text typed for this +// one — silently destroying a conflict in a directory the reader was not even +// looking at. +test('a launch outside any repository cannot write the previous one’s file', async () => { + const repo = conflictedRepo() + const loose = mkdtempSync(join(tmpdir(), 'diffbro-loose-')) + const loosePath = join(loose, 'loose.txt') + writeFileSync( + loosePath, + 'top\n<<<<<<< HEAD\nLOOSE-OURS\n=======\nLOOSE-THEIRS\n>>>>>>> x\nbottom\n' + ) + const userDataDir = freshUserDataDir() + const app = await launchApp(userDataDir) + const page = await firstReadyPage(app) + try { + await runMergetool(userDataDir, repo.dir, repo.path('alpha.txt')) + await expect(page.getByTestId('conflicts-dialog')).toBeVisible({ timeout: 20000 }) + // Arm a row in the repo, then leave it for a file git does not own. + await page.getByTestId('conflict-row-beta.txt').click() + await expect(page.locator('.merge-file')).toHaveText('beta.txt') + + const betaBefore = readFileSync(repo.path('beta.txt'), 'utf8') + await runMergetool(userDataDir, loose, loosePath) + await expect(page.locator('.merge-file')).toHaveText('loose.txt', { timeout: 20000 }) + await page.getByTestId('merge-take-theirs').click() + await page.getByTestId('merge-save').click() + + // The loose file is the one that changes; beta is untouched. + await expect + .poll(() => readFileSync(loosePath, 'utf8'), { timeout: 10000 }) + .toBe('top\nLOOSE-THEIRS\nbottom\n') + expect(readFileSync(repo.path('beta.txt'), 'utf8')).toBe(betaBefore) + } finally { + await app.close().catch(() => {}) + rmSync(repo.dir, { recursive: true, force: true }) + rmSync(loose, { recursive: true, force: true }) + } +}) diff --git a/e2e/merge-many.spec.mjs b/e2e/merge-many.spec.mjs index 585c3d5..c5768c2 100644 --- a/e2e/merge-many.spec.mjs +++ b/e2e/merge-many.spec.mjs @@ -75,6 +75,15 @@ test('walks thirty conflicted files without running out of room', async () => { await mergetool(userDataDir, dir, join(dir, name)) const view = page.locator('.merge-view') await expect(view, `file ${i} (${name}) never opened`).toBeVisible({ timeout: 20000 }) + // The list is offered ONCE. A reader working the walk git's own way puts + // it away on the first file, and it stays away — thirty launches must not + // mean thirty modals. + const list = page.getByTestId('conflicts-dialog') + if (i === 0) { + await expect(list, 'the list never opened').toBeVisible({ timeout: 20000 }) + await page.keyboard.press('Escape') + } + await expect(list, `file ${i} (${name}) pushed the list back`).toHaveCount(0) await expect(page.locator('.merge-file')).toHaveText(name) // git walks one launch at a time, so the band has to say where you are. await expect(page.getByTestId('merge-walk')).toHaveText(`${i + 1} of ${FILES}`) diff --git a/e2e/merge-resolve.spec.mjs b/e2e/merge-resolve.spec.mjs index c95b526..8db19c0 100644 --- a/e2e/merge-resolve.spec.mjs +++ b/e2e/merge-resolve.spec.mjs @@ -189,9 +189,9 @@ test('takes a side, then lets the reader type the answer neither side had', asyn } }) -// The gutter chevrons are the interaction a merge tool is judged on: the side -// you want, moved into the result from where it sits. -test('a gutter chevron moves that side into the result', async () => { +// The take buttons are the interaction a merge tool is judged on: the side you +// want, moved into the result from where it sits. +test('a take button moves that side into the result', async () => { const { dir, file } = conflictedRepo() const userDataDir = freshUserDataDir() const app = await launchApp(userDataDir) @@ -205,9 +205,9 @@ test('a gutter chevron moves that side into the result', async () => { await expect(result).not.toContainText('<<<<<<<') await expect(result).not.toContainText('=======') - const theirChevron = page.locator('.merge-pane').last().locator('.merge-take-theirs') - await expect(theirChevron).toHaveCount(1) - await theirChevron.click() + const theirButton = page.getByTestId('merge-take-theirs-0') + await expect(theirButton).toHaveCount(1) + await theirButton.click() await expect(page.getByTestId('merge-save')).toBeEnabled() await page.getByTestId('merge-save').click() diff --git a/scripts/lib/legacySize.mjs b/scripts/lib/legacySize.mjs index 09c4403..3040f80 100644 --- a/scripts/lib/legacySize.mjs +++ b/scripts/lib/legacySize.mjs @@ -8,7 +8,7 @@ export const LEGACY_SIZE = { 'src/main/files.js': { fn: 84 }, 'src/main/hashDiff.js': { file: 335 }, 'src/main/lineIndexCore.js': { fn: 67 }, - 'src/main/menu.js': { fn: 111 }, + 'src/main/menu.js': { fn: 79 }, 'src/main/quickLook.js': { file: 253 }, 'src/main/sealing.js': { file: 388 }, 'src/main/share.js': { fn: 205, file: 510 }, @@ -23,7 +23,7 @@ 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: 139 }, + 'src/renderer/src/menus.js': { fn: 87 }, 'src/renderer/src/monaco-mermaid.js': { fn: 94 }, 'src/renderer/src/stores/diffStore.js': { file: 747 }, 'src/renderer/src/stores/snippetStore.js': { file: 470 }, diff --git a/scripts/theme-sweep.mjs b/scripts/theme-sweep.mjs index 0eb5208..f388a8b 100644 --- a/scripts/theme-sweep.mjs +++ b/scripts/theme-sweep.mjs @@ -16,7 +16,7 @@ // PNGs land in docs/screenshots/themes/-.png so a human can // still look; the exit code is what gates. -import { mkdtempSync, mkdirSync, existsSync, writeFileSync } from 'node:fs' +import { mkdtempSync, mkdirSync, existsSync, rmSync, writeFileSync } from 'node:fs' import { execFileSync, spawn } from 'node:child_process' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -254,7 +254,98 @@ async function openMerge(page, userDataDir) { await page.locator('.merge-word-theirs').first().waitFor({ timeout: 10000 }) } +// A SECOND repository, with three text conflicts and one binary, so the +// conflicts list has a row in each of its three states to measure. +// +// A FRESH one per theme, unlike mergeConflictRepo's single cached tree: this +// surface ANSWERS a row to produce the settled state, and main keeps its +// resolved set per repository root — so a reused tree would open the second +// theme with that row already done and no button left to press. +const walked = [] +function mergeWalkRepo() { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-sweep-walk-')) + const env = { + ...process.env, + GIT_AUTHOR_NAME: 'T', + GIT_AUTHOR_EMAIL: 't@e', + GIT_COMMITTER_NAME: 'T', + GIT_COMMITTER_EMAIL: 't@e' + } + const git = (...args) => execFileSync('git', args, { cwd: dir, env }) + const names = ['config.yml', 'deploy.yml', 'values.yml'] + const write = (name, replicas) => + writeFileSync(join(dir, name), `service: diff-engine\nreplicas: ${replicas}\n`) + const blob = (byte) => writeFileSync(join(dir, 'logo.png'), Buffer.from([0x89, 0x50, 0x00, byte])) + git('init', '-q', '-b', 'main') + for (const name of names) write(name, 3) + blob(1) + git('add', '.') + git('commit', '-qm', 'base') + git('checkout', '-qb', 'feature') + for (const name of names) write(name, 9) + blob(2) + git('commit', '-qam', 'theirs') + git('checkout', '-q', 'main') + for (const name of names) write(name, 5) + blob(3) + git('commit', '-qam', 'ours') + try { + git('merge', 'feature') + } catch { + // Expected: this is the conflict the surface is for. + } + walked.push(dir) + return { dir, file: join(dir, 'config.yml') } +} + +async function openConflictList(page, userDataDir) { + const { dir, file } = mergeWalkRepo() + const env = { ...process.env } + delete env.ELECTRON_RUN_AS_NODE + await new Promise((resolve) => { + const child = spawn( + ELECTRON, + [MAIN, `--user-data-dir=${userDataDir}`, 'mergetool', file, file, file], + { cwd: dir, env, stdio: 'ignore' } + ) + child.on('exit', resolve) + setTimeout(resolve, 8000) + }) + await page.bringToFront() + await page.getByTestId('conflicts-dialog').waitFor({ timeout: 20000 }) + // One row settled, so the done ink has something to measure. + await page.getByTestId('conflict-row-values.yml').hover() + await page.getByTestId('conflict-theirs-values.yml').click() + await page.locator('.cf-row.done').first().waitFor({ timeout: 10000 }) +} + const SURFACES = [ + { + name: 'conflict-list', + // The row's three status inks are NEW roles (--mg-open / --mg-done / + // --mg-blocked) and each is a WORD, so every one answers to the reading + // floor rather than the 3:1 the --dg-* roles it derives from carry. + open: (page, { userDataDir }) => openConflictList(page, userDataDir), + // The whole merge, not just the dialog: closing the list marks it put away + // for the rest of the walk, so a theme that left one open would find no + // dialog on the next launch. Cancelling ends the walk and resets that. + close: async (page) => { + await page.getByTestId('conflicts-close').click() + await page.getByTestId('conflicts-dialog').waitFor({ state: 'detached' }) + await page.getByRole('button', { name: 'Cancel' }).click() + await page.locator('.merge-view').waitFor({ state: 'detached' }) + }, + probes: { + 'file name': ['.cf-row.open .cf-name', TEXT], + // The directory gives way first, but it is still read: --text-hint, never + // --text-dim, which falls to 2.96 on a hovered row. + directory: ['.cf-row.open .cf-dir', TEXT], + 'still to answer': ['.cf-row.open .cf-state', TEXT], + answered: ['.cf-row.done .cf-state', TEXT], + 'cannot be opened': ['.cf-row.blocked .cf-state', TEXT], + 'files and progress': ['.cf-tally', TEXT] + } + }, { name: 'merge-view', // The chrome only. The bands and the intra-line tints inside the panes are @@ -746,6 +837,11 @@ async function main() { } } finally { await app.close() + // The two scratch repositories the merge surfaces build, which are real + // working trees and not worth leaving in the temp dir. + for (const dir of [conflicted?.dir, ...walked]) { + if (dir) rmSync(dir, { recursive: true, force: true }) + } } report(findings) diff --git a/specs/2026-08-11-merge-conflicts-list/plan.md b/specs/2026-08-11-merge-conflicts-list/plan.md new file mode 100644 index 0000000..a7e2beb --- /dev/null +++ b/specs/2026-08-11-merge-conflicts-list/plan.md @@ -0,0 +1,270 @@ +# Merge conflicts list — the list in front of the merge + +| | | +| --------------------------------------- | --------------------------- | +| **Status** | shipped | +| **Progress** | 14 / 14 steps | +| **Branch** | `feat/merge-conflicts-list` | +| **Started** | 2026-08-11 | +| **Finished** | 2026-08-11 | +| **Bugs found and fixed this iteration** | 13 / 13 | +| **Token baseline** | 2026-08-11T09:43:47Z | +| **Claude tokens used** | 79,949,706 processed | + +## Problem + +`git mergetool` walks conflicted files one launch at a time. DiffBro shows the +file git happens to be asking about and a chip reading `3 of 7` +(`MergeView.vue`, `merge.showsWalk`) — and that is the entire picture the reader +gets. The other six files are invisible: there is no way to see what is left, +no way to choose an order, and no way to take a whole side without opening +three Monaco editors for a file whose answer is "take theirs". + +Two smaller defects fall out of the same place: + +- The take chevron lives in Monaco's glyph margin (`mergeDecorations.js:63`, + `mergePaneOps.takeFromGutter`). A glyph margin exists only on an editor's + **left** edge, so `take theirs` lands beside the result by accident while + `take ours` sits at the far left of the window, two panes from where its text + goes. It is also not keyboard-reachable — it is a CSS mask on a margin, not a + button. +- A merge session can only be entered once. Nothing reopens anything. + +## Solution + +A conflicts list — `BaseDialog`, 720px — in front of the three-way view. Every +unmerged path, its state, and a whole-file `Ours` / `Theirs` per row. Picking a +row opens the three-way view that already exists; saving returns to the list. + +Approved across three revisions of +. + +| option | why not | +| ----------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------- | +| Separate `BrowserWindow` for the list | Every modal in this app is a `BaseDialog`; a second window needs its own CSP, permission handler and `will-navigate` block. User chose this. | +| Recompute regions from the index instead of reading markers | Replaces git's own auto-merge with a different algorithm on the one file the app overwrites. Settled in v0.4.31; unchanged here. | +| Tally from three index reads per file | 200-file lock conflict makes the dialog wait on 600 `git show` calls. Marker scan of the working file instead — recorded below. | +| Per-region `×` (take neither) button | Cut by the user: it is the one answer that discards text, and a hover target on the line being read is the wrong place for it. | +| Keep the glyph-margin chevrons and _add_ inner-edge buttons | Two affordances for one action. Moving them is a rewrite of tested code, accepted deliberately. | + +## Scope + +**In:** the list dialog; whole-file take from the index; out-of-order +resolution; the already-resolved sentinel; three re-entry routes; take controls +moved to the panes' inner edges; three status-ink tokens and +`--btn-edge-strong`. + +**Out:** grouping by directory (earns its keep past ~20 files, additive later); +reconstructing a session after `git mergetool` has exited (git no longer calls +those files unmerged — DiffBro must not pretend to own a walk git has stopped). + +## Design + +Tokens only. New in `tokens.css`: + +| token | value | why | +| ------------------- | ------------------------------------------------------------ | -------------------------------------------------------------------------------------------------- | +| `--mg-open` | `color-mix(in srgb, var(--warning-border) 45%, var(--text))` | raw `--warning-border` as ink scores **2.73** (light) / 2.74 (contrast) — under 4.5 on 9 themes | +| `--mg-done` | `color-mix(in srgb, var(--success-text) 55%, var(--text))` | raw scores 3.09 (solar) / 3.61 (sepia) | +| `--mg-blocked` | `color-mix(in srgb, var(--danger-border) 35%, var(--text))` | `--danger-border` is the weakest role; nord caps the mix at 35% | +| `--btn-edge-strong` | `color-mix(in srgb, var(--border) 25%, var(--text))` | the take button floats on a tinted band where `--btn-edge` lands at 2.99 (sepia) / 3.00 (meridian) | + +Row: 3px status stripe · `` (`git-merge` / `check` / `binary`, all +already in `icons.js`) · name in `--font-mono` `--text` · directory in +`--text-hint` (**not** `--text-dim`, which scores 2.96 on a hovered row) · +tally · hover/focus `Ours` `Theirs` as plain `.btn`. Selection is +`--bg-hover` **plus** a `--btn-edge` keyline — the wash alone is 1.15 on amber. +List well is `--bg` inside the `--bg-panel` dialog. Footer `Close` (ghost, the +dismissive twin) + `Resolve` (primary). + +### Theme verdict — all 20 + +Values parsed from `styles/themes.css`. Each theme's **tightest** pair of the +nine measured; all 20 pass. + +| theme | ground | tightest pair | value | floor | verdict | note | +| -------- | ------ | ------------- | ----- | ----- | ------- | ------------------------------------------------------------- | +| light | light | well keyline | 1.59 | 1.2 | passes | only theme that inverts canvas/raised | +| dark | dark | well keyline | 1.42 | 1.2 | passes | reference palette | +| solar | light | well keyline | 1.29 | 1.2 | passes | `--text-dim` path scored 4.36 → moved to `--text-hint` | +| neon | dark | well keyline | 1.42 | 1.2 | passes | accent `#22d3ee`; no glow anywhere | +| nord | dark | blocked ink | 4.60 | 4.5 | passes | weakest red `#a54c55` caps the mix at 35% | +| sepia | light | done ink | 4.59 | 4.5 | passes | tightest theme in the set on every pair | +| dim | dark | well keyline | 1.33 | 1.2 | passes | | +| beacon | dark | blocked ink | 10.33 | 4.5 | passes | hard keyline `#e0e0e0`; well reads as a framed table at 14.9 | +| meridian | light | well keyline | 1.40 | 1.2 | passes | `--btn-edge` on a band was exactly 3.00 → `--btn-edge-strong` | +| linen | light | well keyline | 1.28 | 1.2 | passes | | +| bloom | light | well keyline | 1.24 | 1.2 | passes | faintest frame; stripe carries the structure | +| nyan | dark | blocked ink | 8.63 | 4.5 | passes | accent `#ff2ecb` never a fill or a label | +| matrix | dark | blocked ink | 8.88 | 4.5 | passes | `--accent` = `--success-text` = `#00ff41`; ring is an outline | +| contrast | light | open ink | 7.78 | 4.5 | passes | hard keyline `#111111`; raw warning ink was 2.74 here | +| volcano | dark | blocked ink | 10.69 | 4.5 | passes | hard keyline `#ffc9a4`; widest margins | +| amber | dark | blocked ink | 7.30 | 4.5 | passes | `--text` is itself gold — icon and word carry the state | +| tide | dark | well keyline | 1.71 | 1.2 | passes | | +| ember | dark | well keyline | 1.46 | 1.2 | passes | | +| graphite | dark | well keyline | 1.52 | 1.2 | passes | near-achromatic accent; status hues are the only colour | +| vector | light | well keyline | 1.64 | 1.2 | passes | | + +## Security rules touched + +**Rule 3 (renderer/main), rule 6 (untrusted input) and rule 7 (fenced exits) — +all three come near, and this is the section that matters.** + +- The renderer **never names a file**. `merge:list` returns entries with a + display name and a directory label; every mutating call takes an **index into + the list main built**. `mergeSession.js` keeps the list; a path is recomputed + in main from the repo root plus that entry. +- Every write **re-verifies at write time**, not at list time: the target must + still appear in `git diff --name-only --diff-filter=U` for the repo this + launch was given. A file that stopped being unmerged between listing and + clicking is refused. +- Stage reads keep `mergeInputs.js`'s existing ceiling (`MAX_STAGE_BYTES`, + 32 MB) and binary sniff. A binary or oversized file is listed as `blocked` + and can only be answered whole-file, never opened as text. +- No new dependency, no network call, no `shell.*`, no crypto. `git` is invoked + through the existing `runGitIn` with `HARDENING` and `gitEnv` (every `GIT_*` + dropped). + +## Test plan + +- **unit** `tests/main/conflictList.test.js` — enumerate, tally, blocked + classification, and the refusal cases: index out of range, entry no longer + unmerged, path escaping the root. +- **unit** `tests/main/mergeSession.test.js` (extend) — the resolved set across + launches; an already-resolved launch writes `written` without a pending + session. +- **unit** `tests/renderer/features/merge/conflictsStore.test.js` — open/close, + selection, marking resolved, remaining count, re-entry availability. +- **unit** `tests/renderer/features/merge/mergeTakeOverlay.test.js` — anchor → + offset mapping for the inner-edge buttons. +- **e2e** `e2e/merge-conflicts-list.spec.mjs` — real `git mergetool` over a + 3-file conflict: the list opens first; resolve the _third_ file first; the + relaunch for it never shows a view; reopen the list from the walk chip. +- **red → green** — the two defects fixed here (`takeFromGutter` not + keyboard-reachable, no route back after dismissing) each get their failing + test watched first. + +## Docs impact + +| surface | needed? | what changes | +| ------------------------ | ------- | -------------------------------------------------------------------------------------------------------------------- | +| `README.md` | yes | merge flow is a headline capability; the description says "one file at a time" implicitly | +| `docs/screenshots/*.png` | no | none of the five captured states (`empty-state`, `diff-*`, `save-encrypted`, `spreadsheet-diff`) show the merge view | +| `docs/roadmap.md` | yes | merge track gains this item as done | +| `docs/brand/roadmap.svg` | yes | same move, hand-authored alongside | +| `docs/security.md` | yes | rule 7's merge paragraph — main now writes more than one file per launch, under a verified list | +| `docs/ipc-security.md` | yes | three new handlers, all index-addressed | + +## Implementation plan + +- [x] 1. `src/main/conflictList.js` — pure core + tests, red first +- [x] 2. `mergeSession.js` — resolved set, list custody, already-resolved sentinel +- [x] 3. `cliRoute.js` — build the list on a merge launch; short-circuit a resolved file +- [x] 4. IPC + preload — `merge:list`, `merge:takeSide`, `merge:openIndex` +- [x] 5. `tokens.css` — the four tokens +- [x] 6. `conflictsStore.js` +- [x] 7. `ConflictsDialog.vue` + `ConflictRow.vue` + styles +- [x] 8. Re-entry: walk chip button, toolbar chip, menu entries, `commands.js` row +- [x] 9. Take controls → pane inner edges (`useMergeTakeOverlay.js`), glyph margin removed +- [x] 10. `en.json` + `node scripts/pseudolocale.mjs` +- [x] 11. Unit tests to the coverage floor +- [x] 12. `e2e/merge-conflicts-list.spec.mjs` +- [x] 13. `theme-sweep.mjs` — `conflict-list` surface +- [x] 14. Docs rows above + +## Decisions + +| date | decision | why | rejected | +| ---------- | ----------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | ------------------------------------- | +| 2026-08-11 | Tally comes from a marker scan of the working file | Three index reads per file blocks the dialog on a large conflict; the count is a scanning aid, not a contract | index-derived tally | +| 2026-08-11 | Renderer addresses files by list index, never by path | Rule 3/7 — main has held the merge path since launch and must keep holding it | passing a repo-relative path over IPC | +| 2026-08-11 | Write-time re-verification against `--diff-filter=U` | The list can go stale between opening and clicking; a stale entry must fail closed | trusting the list snapshot | +| 2026-08-11 | Closing the dialog does not end the merge session | One owner for that decision (Save/Cancel in the three-way view), so a dismissed modal cannot end a merge | `Later` cancelling the current file | +| 2026-08-11 | Take controls move rather than duplicate | Two affordances for one action is worse than a rewrite | keeping the glyph-margin chevrons | + +## Validation + +- [x] `npm run check` — **exit 0**. `eslint . --max-warnings 0` clean; style, + theme-depth, structure, i18n and rawtext gates clean; 3348 tests passing; + coverage 95.41 / 88.42 / 95.82 / 96.48 against floors 95 / 88 / 95 / 96. +- [x] `make e2e` — **exit 0**, 483 passed (5.7m), including the six new + `merge-conflicts-list` specs and the 19 in `merge-resolve`. +- [x] `make theme-sweep SWEEP_ONLY=conflict-list` — **exit 0**, 120 measurements + across 20 themes, tightest sepia `answered` at 4.61 against a 4.5 floor. + Screenshots in `docs/screenshots/themes/conflict-list-*.png`. +- [x] UI seen running — the sweep drives a real `git mergetool` over a four-file + conflict (three text, one binary) in the container and screenshots all 20. +- [x] every Docs-impact "yes" done — README feature row, roadmap mermaid + prose, + `docs/brand/roadmap.svg`, `docs/security.md` (new subsection), + `docs/ipc-security.md` (two rows). +- [x] token usage measured — table below. + +### Audit round — reviewer + QA agents + +Two agents were run over the finished change, one reading and one executing. +The gates they re-ran reproduced exactly (`npm run check` 0, `make e2e` 483, +sweep 120/20). Everything below is what they found BEYOND that, all fixed here. + +| # | severity | what | found by | fix | +| --- | ----------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | +| 5 | **data loss** | A mergetool launch for a file in NO repository left the previous repository's row armed, so the next Save wrote **that** repo's file with text typed for this one — silently destroying its conflict. | QA, by driving a loose file after a repo | `walkFor` ends the conflict session when `locate()` finds no repo (`mergeRoute.js`) | +| 6 | **hang** | The primary **Resolve** button opens the launch's OWN pre-selected row, and `saveMerge` routed on `openRel` alone — writing the file but never releasing the launcher. The terminal then waited out its full two hours, and every later launch for that file hung with no window at all. | both agents, independently | `saveMerge` compares `openRel` to `launchRel` and falls through to the launch's own fence | +| 7 | behaviour | Cancelling while an out-of-order row was open wrote `cancelled` for the LAUNCH's file — telling git a file the reader never looked at was declined. | reviewer | `merge:cancel` backs out of the row only when it is not the launch's | +| 8 | **shortcut regression** | `CmdOrCtrl+Shift+M` was already Tools ▸ XML. The new View item registered first, silently killing it, and the Shortcuts sheet still advertised the old owner. | reviewer | moved to `CmdOrCtrl+Shift+K`, advertised in `shortcuts.js`, and a new **uniqueness test** over `buildMenus` (proven red against the collision) | +| 9 | false promise | `"Binary — pick a side"` could never work: `runGitIn` returns stdout as a String, so the stage was already a lossy UTF-8 decode and the binary sniff rejected every click. | both agents | new `runGitBytesIn`; stages read and written as **bytes** | +| 10 | unusable | A non-ASCII filename came back from git quoted (`"\303\274…"`), so the row showed octal, was misclassified as binary, and the launch's own file was never found in the list. | QA | `-z` on `unmergedArgs()` + `splitNulPaths` | +| 11 | resource | The size cap was checked on the Buffer — after an unbounded `readFileSync` had already pulled the whole file into main. | reviewer | `statSync` first; `entryFor`'s existing `size` parameter is now actually passed | +| 12 | UX / convention | The list re-opened on **every** launch, so a thirty-file walk meant thirty modals; and `.merge-take-btn` was a fourth hand-rolled 19px control box. | reviewer, and noted before the audit | `dismissed` state (put away once, stays away for the walk); button sized from `--chip-h` | +| 13 | **broken on Windows** | `locate()` derived the repo-relative path with `path.relative` over realpath'd ends. Windows hands out an 8.3 temp path (`RUNNER~1`) where git answers with the long one, so the result was a `..`-laden path the fence rejected — **the conflicts list never opened on Windows at all**, silently falling back to the old single-file view. | Windows CI (`check (windows-latest)`); the macOS host and the Linux container both missed it | git names the file itself — `ls-files --full-name -z`; the computation stays only as a fallback for an untracked file | + +Also closed: the conflict session now ends when the walk finishes or the reader +declines, so `merge:take` / `merge:open` stop being live write handlers into +that repository — `conflicts.end()` was previously dead code its own test +guarded. `conflictSession.js` gained `tests/main/conflictSession.test.js` and +joined the coverage set: it holds both write paths and had none. + +**What the reviewer verified clean** (so the next run need not re-tread): all +eight hard security rules, including no path by which a renderer-supplied value +reaches `writeFileSync` — every write is `join(session.root, entry.rel)` with +`rel` from git's own output through `isRepoRelative`, re-verified against +`--diff-filter=U` on both write paths; every ratchet (both `legacySize` entries +genuinely beaten, not relaxed); slice boundaries; i18n extraction. + +**Left open, deliberately:** `join(root, rel)` is not realpath-contained, so a +symlinked directory component inside a hostile repository could put a write +outside the root. git's own checkout protections make it hard to reach and no +agent could; recorded rather than fixed under a release. + +### Bugs found and fixed while building (before the audit) + +| # | what | found by | fix | +| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------- | -------------------------------------------------------------------------------- | +| 1 | `routeMerge` ran the marker guard BEFORE the already-answered check, so a relaunch for a file resolved out of order was refused with **no sentinel** — hanging the terminal for the launcher's full two hours. The comment above `walkFor` claimed the opposite of what the code did. | `e2e/merge-conflicts-list.spec.mjs` — the one test written for exactly this | walk lookup first, marker guard moved inside `show()` | +| 2 | The row's Ours/Theirs used `visibility: hidden`, which takes the buttons **out of the tab order** — so the `:focus-within` rule beside it could never fire and the only way to answer a row was the mouse. | e2e click timing out, then reading the CSS | `opacity: 0`, which keeps them focusable | +| 3 | The walk chip read **`6 of 6`** for a four-file conflict: `walkPosition` counts launches this PROCESS has seen, which stops being a position once a second `git mergetool` run happens. | the theme-sweep screenshot | position/total now come from the list (`walkOf`) | +| 4 | Answering the launch's OWN file from its row wrote the file but left the launch session armed and the launcher waiting on a sentinel that would never come. | reading the flow while wiring `merge:take` | the handler spends the session and releases it when the row is the launch's file | + +Two ratchets moved, both **downward**: adding a View-menu item pushed +`installMenu` to 116 and `buildMenus` to 144 lines. Rather than raise their +`legacySize` entries, the View submenus were extracted into `viewMenu` / +`viewSection`, which took the functions to **79** and **87** — retightened with +`--retighten`, never raised. + +### Token usage + +| category | tokens | +| ----------- | -------------: | +| input | 428 | +| output | 116,302 | +| cache write | 220,100 | +| cache read | 79,612,876 | +| **total** | **79,949,706** | + +Cache read dominates: it is context re-sent each turn at a fraction of fresh +input, so the total is tokens _processed_, not a cost. + +**Outcome:** shipped. `git mergetool` now opens on the list of every conflicted +file; a row can be answered whole-file from the index or opened in the three-way +view, in any order; the take controls are real buttons on the panes' inner +edges; and dismissing the list no longer ends anything — the walk chip, a +toolbar chip and View ▸ Merge conflicts… all bring it back. diff --git a/src/main/cliRoute.js b/src/main/cliRoute.js index 6d376ca..f991fd3 100644 --- a/src/main/cliRoute.js +++ b/src/main/cliRoute.js @@ -11,11 +11,7 @@ import { gitToolStatus, registerGitTool, sweepGitTemp, unregisterGitTool } from import { ensureMainWindow } from './quickLook' import { allowCliPath } from './files' import { fileAtRevision, isRevisionSide, REVISION_ERROR_KEYS } from './gitCliFiles' -import { beginMerge, cancelMerge, writeMerged } from './mergeSession' -import { mergeInputsFor } from './mergeInputs' -import { readFileSync } from 'node:fs' -import { basename } from 'node:path' -import { hasConflictMarkers, isBinaryBuffer } from './mergeGuards' +import { registerMergeIpc, routeMerge } from './mergeRoute' import { t } from './i18n' // A command can arrive before any window exists (a cold `diffbro compare …`), @@ -86,48 +82,12 @@ function deliverResolved(command, cwd) { const needsGit = (command) => command?.name === 'compare' && command.files.some(isRevisionSide) -// A mergetool launch: main REMEMBERS the path git wants written and sends the -// renderer the conflicted text, never the path. What comes back is text. -function routeMerge(command) { - let buffer - try { - buffer = readFileSync(command.merged) - } catch { - process.stderr.write(`${t('cliErrors.merge-unreadable')}\n`) - return - } - // git calls the mergetool for a BINARY conflict too, and leaves it with no - // markers. Decoding one as text turns every invalid byte into U+FFFD, and - // writing that back destroys the file — so it never reaches the renderer. - if (isBinaryBuffer(buffer)) { - process.stderr.write(`${t('cliErrors.merge-binary')}\n`) - return - } - const content = buffer.toString('utf8') - // Nothing to decide is not the same as "resolved": a file with no markers is - // one this tool has no business rewriting. - if (!hasConflictMarkers(content)) { - process.stderr.write(`${t('cliErrors.merge-no-conflicts')}\n`) - return - } - beginMerge(command) - // A NAME for the band, never a path: the renderer must not be able to learn, - // or name, the file main is going to write. - const base = { name: 'merge', fileName: basename(command.merged), content } - mergeInputsFor(command.merged) - .then((inputs) => deliver({ ...base, ...inputs })) - // Without this the renderer is never told, no sentinel is written, and the - // terminal that ran `git mergetool` waits forever on a decision nobody was - // asked for. The markers alone are enough to resolve it. - .catch(() => deliver(base)) -} - // The verbs that need something done before the renderer hears about them. const SPECIAL = { // `open` with no file has nothing to tell the renderer — the window IS the // answer, so it never reaches deliver's pending queue. raise: () => void ensureMainWindow()?.focus(), - merge: (command) => routeMerge(command), + merge: (command) => routeMerge(command, deliver), 'clipboard-save': (command) => deliver({ ...command, text: clipboard.readText() }) } @@ -174,11 +134,7 @@ export function registerCliIpc() { }) ).response === 1 - // The renderer's ONLY say in the merge is the text. It cannot name a file: - // main has held that path since the launch. - ipcMain.handle('merge:write', (e, text) => writeMerged(text)) - // Declining is an answer too: it releases the launcher and spends the session. - ipcMain.handle('merge:cancel', () => cancelMerge()) + registerMergeIpc() ipcMain.handle('cli:status', () => shimStatus(where())) ipcMain.handle('cli:install', async () => (await confirmed(t('dialog.cliInstall.message'), t('dialog.cliInstall.detail'))) diff --git a/src/main/conflictList.js b/src/main/conflictList.js new file mode 100644 index 0000000..9d62d4d --- /dev/null +++ b/src/main/conflictList.js @@ -0,0 +1,87 @@ +// The conflicted files git is walking, as a list the reader can choose from. +// +// Pure: git output and file contents arrive as arguments, so every refusal is +// testable without a repository. The paths in here NEVER reach the renderer — +// it addresses a row by its index and main resolves that back to a path, which +// is what keeps the merge write fenced (docs/security.md rule 7). +import { posix } from 'node:path' +import { isBinaryBuffer, MAX_MERGE_BYTES } from './mergeGuards' + +// mergeGuards' rule, in counting form — so a row's number and the view's count +// of regions cannot disagree. +const OPENS = /^<<<<<<<(?: |$)/gm + +/** How many regions the working copy still shows. */ +export function conflictTally(text) { + return String(text ?? '').match(OPENS)?.length ?? 0 +} + +/** + * A path from git's own output, and nothing that could leave the repository or + * read as an option. Deliberately narrower than git's rules. + * @param {unknown} rel + */ +export function isRepoRelative(rel) { + if (typeof rel !== 'string' || !rel) return false + if (rel.startsWith('/') || rel.startsWith('-')) return false + const parts = rel.replace(/\\/g, '/').split('/') + return !parts.includes('..') && !parts.includes('') +} + +const dirOf = (rel) => { + const dir = posix.dirname(rel) + return dir === '.' ? '' : dir +} + +const blocked = (rel) => ({ + rel, + name: posix.basename(rel), + dir: dirOf(rel), + tally: 0, + state: 'blocked' +}) + +/** + * One row. A file that cannot be opened as text — binary, past the ceiling, + * unreadable, or with no markers left in it — is still a row, because hiding it + * would make the list disagree with what git calls unmerged. It is `blocked`: + * answerable whole-file from the index, never opened as text. + * + * @param {{rel: string, buffer: Buffer|null, size?: number}} file + */ +export function entryFor({ rel, buffer, size }) { + if (!isRepoRelative(rel)) return null + const bytes = size ?? buffer?.byteLength ?? 0 + if (!buffer || bytes > MAX_MERGE_BYTES || isBinaryBuffer(buffer)) return blocked(rel) + const tally = conflictTally(buffer.toString('utf8')) + if (!tally) return blocked(rel) + return { rel, name: posix.basename(rel), dir: dirOf(rel), tally, state: 'open' } +} + +/** + * The list, in git's own order. + * @param {object} source + * @param {string[]} source.paths + * @param {(rel: string) => {size: number, buffer: Buffer|null}} source.read + * sized before it is read, so the cap bounds what reaches memory + * @param {Set} [source.resolved] + */ +export function visibleEntries({ paths, read, resolved }) { + const out = [] + for (const rel of paths) { + const entry = entryFor({ rel, ...read(rel) }) + if (!entry) continue + out.push(resolved?.has(rel) ? { ...entry, tally: 0, state: 'done' } : entry) + } + return out +} + +/** + * The row the renderer named, or null. Every shape that is not an index into + * this list fails closed — an index is the renderer's ONLY say in which file + * gets written. + */ +export function pickEntry(entries, index) { + if (!Number.isInteger(index) || index < 0 || index >= entries.length) return null + return entries[index] +} diff --git a/src/main/conflictSession.js b/src/main/conflictSession.js new file mode 100644 index 0000000..e44950e --- /dev/null +++ b/src/main/conflictSession.js @@ -0,0 +1,213 @@ +// Custody of the conflict list for one `git mergetool` run. +// +// git walks the conflicted files one launch at a time and only MAIN survives +// between them, so the list, the repository root and what has already been +// answered are remembered here. The renderer addresses a file by its INDEX into +// this list and never by name: main resolves the index back to a path, which is +// what keeps a merge write fenced (docs/security.md rule 7). +import { readFileSync, realpathSync, statSync, writeFileSync } from 'node:fs' +import { dirname, join, relative, sep } from 'node:path' +import { pickEntry, isRepoRelative, visibleEntries } from './conflictList' +import { + readStageArgs, + repoPathOfArgs, + repoRootArgs, + runGitBytesIn, + runGitIn, + splitNulPaths, + unmergedArgs +} from './gitRepo' +import { stagesFor } from './mergeInputs' +import { isBinaryBuffer, MAX_MERGE_BYTES } from './mergeGuards' + +let session = null + +// Through realpath first: on macOS a repo under /var is reported by git as +// /private/var, and relative() between the two forms yields a path full of `..` +// that the fence then rejects. +const real = (path) => { + try { + return realpathSync(path) + } catch { + return path + } +} + +/** + * The repository holding a mergetool's `$MERGED`, and where that file sits in + * it. Null when git will not claim it — a tool invoked by hand, a file outside + * any repo — which leaves the single-file path exactly as it was. + * + * git names the file itself (`ls-files --full-name`); computing it from + * `path.relative` needs both ends in the same form and they are not. The + * computation stays only as a fallback for a file git does not track. + */ +export async function locate(merged) { + const where = dirname(merged) + const res = await runGitIn(repoRootArgs(), where) + const root = res.ok ? res.stdout.trim() : '' + if (!root) return null + const named = await runGitIn(repoPathOfArgs(merged), where) + const rel = named.ok + ? (splitNulPaths(named.stdout)[0] ?? '') + : relative(real(root), real(merged)).split(sep).join('/') + return isRepoRelative(rel) ? { root, rel } : null +} + +// Sized BEFORE it is read: the cap is there to stop a huge conflicted file +// being pulled into main, and checking it on the resulting Buffer would already +// have pulled it. `size` is what entryFor classifies on. +const readAt = (root) => (rel) => { + try { + const path = join(root, rel) + const size = statSync(path).size + return { size, buffer: size > MAX_MERGE_BYTES ? null : readFileSync(path) } + } catch { + return { size: 0, buffer: null } + } +} + +const unmergedPaths = async (root) => { + const res = await runGitIn(unmergedArgs(), root) + return res.ok ? splitNulPaths(res.stdout).filter(isRepoRelative) : [] +} + +/** + * Build (or rebuild) the list for a repository. Called on every launch, so a + * file answered outside the app between two launches drops out on its own. + * @param {string} root the repository root, computed in main + */ +export async function openConflictSession(root) { + const resolved = session?.root === root ? session.resolved : new Set() + const paths = await unmergedPaths(root) + session = { + root, + resolved, + openRel: null, + entries: visibleEntries({ paths, read: readAt(root), resolved }) + } + return session.entries.length +} + +export function endConflictSession() { + session = null +} + +/** What the renderer is allowed to see: no paths, only what a row displays. */ +export function conflictRows() { + return (session?.entries ?? []).map(({ name, dir, tally, state }) => ({ + name, + dir, + tally, + state + })) +} + +/** Where a path sits in the list, so the launch's own file can be marked. */ +export function indexOfRel(rel) { + return (session?.entries ?? []).findIndex((entry) => entry.rel === rel) +} + +export function isResolvedRel(rel) { + return !!session?.resolved.has(rel) +} + +/** + * Re-verified at USE time, never at list time: a file can stop being unmerged + * between the dialog opening and a row being clicked, and a stale row must fail + * closed rather than write over something git no longer calls conflicted. + */ +async function verifiedEntry(index) { + const entry = pickEntry(session?.entries ?? [], index) + if (!entry) return null + const paths = await unmergedPaths(session.root) + return paths.includes(entry.rel) ? entry : null +} + +// Bytes, never a decoded string: a binary row promises "pick a side", and a +// side that came back through UTF-8 would be written back corrupted. +const stageBytes = async (root, rel, stage) => { + const res = await runGitBytesIn(readStageArgs(stage, rel), root) + if (!res.ok || res.stdout.byteLength > MAX_MERGE_BYTES) return null + return res.stdout +} + +function markResolved(entry) { + session.resolved.add(entry.rel) + const row = session.entries.find((e) => e.rel === entry.rel) + if (row) Object.assign(row, { state: 'done', tally: 0 }) +} + +/** + * The file THIS launch was given, once its own fence has written it. Marked + * here too so the list agrees with what happened, and so a re-launch for it + * short-circuits like any other answered row. + */ +export function noteResolved(rel) { + if (session && rel) markResolved({ rel }) +} + +/** + * Answer a whole file from the index without opening it — the row's Ours / + * Theirs. Stage 2 is ours, 3 theirs. + * @param {{index: number, side: 'ours'|'theirs'}} choice + */ +export async function takeSideAt({ index, side }) { + const entry = await verifiedEntry(index) + if (!entry) return { ok: false, error: 'gone' } + const bytes = await stageBytes(session.root, entry.rel, side === 'theirs' ? 3 : 2) + if (bytes === null) return { ok: false, error: 'unreadable' } + try { + writeFileSync(join(session.root, entry.rel), bytes) + } catch (err) { + return { ok: false, error: String(err?.message ?? err) } + } + markResolved(entry) + return { ok: true, rel: entry.rel } +} + +/** + * The three-way payload for a row, so a file OTHER than the one this launch was + * given can be opened. Main remembers which row is open; that is what a later + * write targets. + */ +export async function openConflictAt(index) { + const entry = await verifiedEntry(index) + if (!entry || entry.state === 'blocked') return { ok: false, error: 'blocked' } + const { buffer } = readAt(session.root)(entry.rel) + if (!buffer || isBinaryBuffer(buffer)) return { ok: false, error: 'blocked' } + session.openRel = entry.rel + return { + ok: true, + fileName: entry.name, + content: buffer.toString('utf8'), + position: index + 1, + total: session.entries.length, + ...(await stagesFor(session.root, entry.rel)) + } +} + +/** The row the view is currently editing, when it is not the launch's own file. */ +export function openRel() { + return session?.openRel ?? null +} + +export function clearOpenRel() { + if (session) session.openRel = null +} + +/** Write a row the launch did not name. Verified again on the way in. */ +export async function writeConflictText(text) { + if (!session?.openRel || typeof text !== 'string') return { ok: false, error: 'no-merge' } + const index = indexOfRel(session.openRel) + const entry = await verifiedEntry(index) + if (!entry) return { ok: false, error: 'gone' } + try { + writeFileSync(join(session.root, entry.rel), text, 'utf8') + } catch (err) { + return { ok: false, error: String(err?.message ?? err) } + } + markResolved(entry) + session.openRel = null + return { ok: true } +} diff --git a/src/main/gitRepo.js b/src/main/gitRepo.js index 9b2b330..b0a1486 100644 --- a/src/main/gitRepo.js +++ b/src/main/gitRepo.js @@ -69,11 +69,38 @@ export function resolveRevisionArgs(rev) { // pattern, only a range. const STAGES = [1, 2, 3] -/** Every file still conflicted, in git's own order. */ +/** + * Every file still conflicted, in git's own order. + * + * `-z` is not a nicety: without it git QUOTES a path holding a non-ASCII byte + * (`"\303\274..."`), and that octal string then travels as if it were the name — + * listed as a row nothing matches, and never found again in the index. + */ export function unmergedArgs() { - return [...HARDENING, 'diff', '--name-only', '--diff-filter=U', '--no-color'] + return [...HARDENING, 'diff', '--name-only', '--diff-filter=U', '--no-color', '-z'] } +/** + * Where a file sits inside its repository, spelled by git rather than computed. + * + * Deriving it from `path.relative` needs both ends in the same form, and they + * are not: Windows hands out an 8.3 temp path (`RUNNER~1`) where git answers + * with the long one, and macOS reports `/private/var` for a repo under `/var`. + * Either mismatch yields a `..`-laden path the fence then rejects — which on + * Windows meant the conflicts list never opened at all. + * + * The path is one main has held since the launch argv, and it goes after `--`. + */ +export function repoPathOfArgs(file) { + return [...HARDENING, 'ls-files', '--full-name', '-z', '--error-unmatch', '--', file] +} + +/** git's `-z` output: NUL-terminated, so the last field is empty. */ +export const splitNulPaths = (stdout) => + String(stdout ?? '') + .split('\0') + .filter(Boolean) + /** * Read one stage of a conflicted file out of the index: 1 ancestor, 2 ours, * 3 theirs. Cleaner than scraping markers, and the only way to the ancestor. @@ -155,3 +182,22 @@ export function runGitIn(args, cwd) { ) }) } + +/** + * The same invocation, with stdout as BYTES. A blob read back through the + * string form above is already a lossy UTF-8 decode — every invalid byte + * replaced — so writing it back would destroy the file it came from. Anything + * that lands on disk unchanged has to come through here. + * @returns {Promise<{ok: boolean, stdout: Buffer}>} + */ +export function runGitBytesIn(args, cwd) { + return new Promise((resolve) => { + execFile( + 'git', + args, + { cwd, windowsHide: true, maxBuffer: 64 * 1024 * 1024, encoding: 'buffer', env: gitEnv() }, + (err, stdout) => + resolve({ ok: !err, stdout: Buffer.isBuffer(stdout) ? stdout : Buffer.alloc(0) }) + ) + }) +} diff --git a/src/main/i18n.js b/src/main/i18n.js index 77892e4..93aa49f 100644 --- a/src/main/i18n.js +++ b/src/main/i18n.js @@ -5,7 +5,12 @@ // verb-or-noun phrase: it appears hundreds of times inside menu templates, and // every reader of any i18n codebase already knows what it returns. import { app } from 'electron' -import { DEFAULT_LOCALE, createTranslator, normalizeLocale, preferredLocale } from '../shared/i18n/index.js' +import { + DEFAULT_LOCALE, + createTranslator, + normalizeLocale, + preferredLocale +} from '../shared/i18n/index.js' let active = DEFAULT_LOCALE let translate = createTranslator(active) diff --git a/src/main/menu.js b/src/main/menu.js index cc76f3a..7665315 100644 --- a/src/main/menu.js +++ b/src/main/menu.js @@ -101,6 +101,45 @@ const settingsItem = (send) => ({ click: () => send('settings') }) +const viewMenu = (send, isDev) => ({ + label: t('menu.view.title'), + submenu: [ + { + label: t('menu.view.commandPalette'), + accelerator: 'CmdOrCtrl+Shift+P', + click: () => send('command-palette') + }, + { + label: t('menu.view.toggleSidebar'), + accelerator: 'CmdOrCtrl+B', + click: () => send('toggle-sidebar') + }, + { type: 'separator' }, + ...displayToggles(send), + { + label: t('menu.view.toggleTheme'), + accelerator: 'CmdOrCtrl+D', + click: () => send('toggle-theme') + }, + { type: 'separator' }, + { + label: t('menu.view.quickLook'), + // User-configurable global shortcut (Settings), so no fixed hint here. + click: () => toggleQuickLook() + }, + { + label: t('menu.view.mergeConflicts'), + accelerator: 'CmdOrCtrl+Shift+K', + click: () => send('merge-conflicts') + }, + { type: 'separator' }, + zoomItem('zoomIn', 'CmdOrCtrl+=', 'zoom-in'), + zoomItem('zoomOut', 'CmdOrCtrl+-', 'zoom-out'), + zoomItem('resetZoom', 'CmdOrCtrl+0', 'zoom-reset'), + ...(isDev ? [{ type: 'separator' }, { role: 'toggleDevTools' }] : []) + ] +}) + export function installMenu() { const isMac = process.platform === 'darwin' disableInjectedMacMenuItems() @@ -169,39 +208,7 @@ export function installMenu() { }, editMenu(sendToFocused, isMac), terminalMenu(sendToFocused), - { - label: t('menu.view.title'), - submenu: [ - { - label: t('menu.view.commandPalette'), - accelerator: 'CmdOrCtrl+Shift+P', - click: () => sendToFocused('command-palette') - }, - { - label: t('menu.view.toggleSidebar'), - accelerator: 'CmdOrCtrl+B', - click: () => sendToFocused('toggle-sidebar') - }, - { type: 'separator' }, - ...displayToggles(sendToFocused), - { - label: t('menu.view.toggleTheme'), - accelerator: 'CmdOrCtrl+D', - click: () => sendToFocused('toggle-theme') - }, - { type: 'separator' }, - { - label: t('menu.view.quickLook'), - // User-configurable global shortcut (Settings), so no fixed hint here. - click: () => toggleQuickLook() - }, - { type: 'separator' }, - zoomItem('zoomIn', 'CmdOrCtrl+=', 'zoom-in'), - zoomItem('zoomOut', 'CmdOrCtrl+-', 'zoom-out'), - zoomItem('resetZoom', 'CmdOrCtrl+0', 'zoom-reset'), - ...(isDev ? [{ type: 'separator' }, { role: 'toggleDevTools' }] : []) - ] - }, + viewMenu(sendToFocused, isDev), securityMenu(sendToFocused), toolsMenu(sendToFocused), helpMenu({ diff --git a/src/main/mergeGuards.js b/src/main/mergeGuards.js index 2813e29..e7ec07f 100644 --- a/src/main/mergeGuards.js +++ b/src/main/mergeGuards.js @@ -1,6 +1,14 @@ // What a mergetool run refuses before the renderer ever sees it. Pure, so both // refusals are testable without a repository. +/** + * The ceiling on anything a merge decodes — a stage read, a working copy + * scanned for a tally. execFile's maxBuffer is 64 MB and would otherwise be the + * only bound, with three decoded copies going over IPC into Monaco models while + * the launcher waits on a decision. + */ +export const MAX_MERGE_BYTES = 32 * 1024 * 1024 + /** * A NUL byte in the first 8 KB means this is not text — the same sniff * files.js uses. git calls the mergetool for a binary conflict as readily as a diff --git a/src/main/mergeInputs.js b/src/main/mergeInputs.js index 25132ee..49d8a8f 100644 --- a/src/main/mergeInputs.js +++ b/src/main/mergeInputs.js @@ -3,20 +3,22 @@ // the two sides go by, and where this file sits in the walk. import { realpathSync } from 'node:fs' import { dirname, relative } from 'node:path' -import { readStageArgs, repoRootArgs, revisionNameArgs, runGitIn, unmergedArgs } from './gitRepo' +import { + readStageArgs, + repoRootArgs, + revisionNameArgs, + runGitIn, + splitNulPaths, + unmergedArgs +} from './gitRepo' import { walkPosition } from './mergeSession' -import { isBinaryBuffer } from './mergeGuards' - -// The same ceiling an opened file gets. A stage read has no other bound — -// execFile's maxBuffer is 64 MB — and three of them are decoded, sent over IPC -// and set as Monaco models, with the launcher already waiting on a decision. -const MAX_STAGE_BYTES = 32 * 1024 * 1024 +import { isBinaryBuffer, MAX_MERGE_BYTES } from './mergeGuards' // A stage that is too big, or not text, sends the view back to reconstructing // the sides from the markers — which is the designed fallback, not a failure. const usableStage = (res) => res.ok && - Buffer.byteLength(res.stdout, 'utf8') <= MAX_STAGE_BYTES && + Buffer.byteLength(res.stdout, 'utf8') <= MAX_MERGE_BYTES && !isBinaryBuffer(Buffer.from(res.stdout, 'utf8')) // A branch name and nothing else: `name-rev` answers `main~2` for a commit that @@ -51,35 +53,45 @@ async function revisionNames(dir) { } /** - * The three inputs git already holds. Absent — a mergetool invoked by hand, a - * file with no index entry — the view falls back to reconstructing the sides - * from the markers themselves. - * @returns {Promise} merged into the renderer's payload + * The three stages and the two branch names, for a file named by the repository + * that holds it. Empty when git cannot give both sides — the view then falls + * back to reconstructing them from the markers, which is the designed fallback. + * @param {string} dir the repository root, computed in main + * @param {string} rel repo-relative, posix */ -export async function mergeInputsFor(merged) { - const root = await runGitIn(repoRootArgs(), dirname(merged)) - if (!root.ok || !root.stdout.trim()) return {} - const dir = root.stdout.trim() - // Through realpath first: on macOS a repo under /var is reported by git as - // /private/var, and relative() between the two forms yields a path full of - // `..` that the fence then rejects — the index read failed silently and the - // view fell back to reconstructing the sides from the markers. - const rel = relative(real(dir), real(merged)) +export async function stagesFor(dir, rel) { try { const [base, ours, theirs] = await Promise.all( [1, 2, 3].map((stage) => runGitIn(readStageArgs(stage, rel), dir)) ) if (!usableStage(ours) || !usableStage(theirs)) return {} - const [unmerged, names] = await Promise.all([runGitIn(unmergedArgs(), dir), revisionNames(dir)]) - const files = unmerged.ok ? unmerged.stdout.split('\n').filter(Boolean).length : 0 return { ours: ours.stdout, theirs: theirs.stdout, base: usableStage(base) ? base.stdout : null, - ...names, - ...walkPosition(files, merged) + ...(await revisionNames(dir)) } } catch { return {} } } + +/** + * The three inputs git already holds, for the file THIS launch was given, plus + * where it sits in the walk. + * @returns {Promise} merged into the renderer's payload + */ +export async function mergeInputsFor(merged) { + const root = await runGitIn(repoRootArgs(), dirname(merged)) + if (!root.ok || !root.stdout.trim()) return {} + const dir = root.stdout.trim() + // Through realpath first: on macOS a repo under /var is reported by git as + // /private/var, and relative() between the two forms yields a path full of + // `..` that the fence then rejects — the index read failed silently and the + // view fell back to reconstructing the sides from the markers. + const stages = await stagesFor(dir, relative(real(dir), real(merged))) + if (!stages.ours) return {} + const unmerged = await runGitIn(unmergedArgs(), dir) + const files = unmerged.ok ? splitNulPaths(unmerged.stdout).length : 0 + return { ...stages, ...walkPosition(files, merged) } +} diff --git a/src/main/mergeRoute.js b/src/main/mergeRoute.js new file mode 100644 index 0000000..c274c5e --- /dev/null +++ b/src/main/mergeRoute.js @@ -0,0 +1,166 @@ +// A `git mergetool` launch, from argv to the message the renderer gets, plus +// the handlers that answer it. Split out of cliRoute.js: this is the one place +// the app writes files it did not create, and it earns its own module. +import { ipcMain } from 'electron' +import { readFileSync } from 'node:fs' +import { basename } from 'node:path' +import { + clearOpenRel, + conflictRows, + endConflictSession, + indexOfRel, + isResolvedRel, + locate, + noteResolved, + openConflictAt, + openConflictSession, + openRel, + takeSideAt, + writeConflictText +} from './conflictSession' +import { + beginMerge, + cancelMerge, + endMerge, + mergeInProgress, + releaseLauncher, + writeMerged +} from './mergeSession' +import { mergeInputsFor } from './mergeInputs' +import { hasConflictMarkers, isBinaryBuffer } from './mergeGuards' +import { t } from './i18n' + +const refuse = (key) => void process.stderr.write(`${t(key)}\n`) + +function readMerged(path) { + let buffer + try { + buffer = readFileSync(path) + } catch { + return refuse('cliErrors.merge-unreadable') + } + // git calls the mergetool for a BINARY conflict too, and leaves it with no + // markers. Decoding one as text turns every invalid byte into U+FFFD, and + // writing that back destroys the file. + if (isBinaryBuffer(buffer)) return refuse('cliErrors.merge-binary') + return buffer.toString('utf8') +} + +/** + * The list this launch belongs to, and whether the launch is for a file the + * reader already answered out of order. + * + * Answered here means answered by the READER, out of git's order. Such a file no + * longer has markers in it, so this has to run BEFORE the marker guard: refused + * there it would write no sentinel, and the terminal running `git mergetool` + * would wait out the launcher's two hours on a decision already made. + */ +async function walkFor(merged) { + const found = await locate(merged) + launchRel = found?.rel ?? null + // No repository behind this launch — a mergetool run by hand, a file outside + // any repo. The PREVIOUS walk's list, and the row it had open, must not + // survive into it: left standing, this launch's Save wrote that repository's + // file with text the reader typed for this one. + if (!found) { + endConflictSession() + return { rows: [], at: -1 } + } + await openConflictSession(found.root) + if (isResolvedRel(found.rel)) return { done: true } + return { rows: conflictRows(), at: indexOfRel(found.rel) } +} + +// Where the file this launch was given sits in the repository, so its own write +// can be recorded in the list the same way an out-of-order one is. +let launchRel = null + +/** + * The list decides the position, overriding mergeSession's launch counter. + * That counter tallies how many launches this PROCESS has seen, which is a + * position only while one walk is the only thing that ever happened — a second + * `git mergetool` run had it reading `6 of 6` for a four-file conflict. + */ +const walkOf = (walk) => + walk.rows.length + ? { conflicts: walk.rows, at: walk.at, position: walk.at + 1, total: walk.rows.length } + : { conflicts: walk.rows, at: walk.at } + +/** + * @param {object} command the parsed `mergetool` verb + * @param {(payload: object) => void} deliver + */ +export function routeMerge(command, deliver) { + const content = readMerged(command.merged) + if (typeof content !== 'string') return + // A NAME for the band, never a path: the renderer must not be able to learn, + // or name, the file main is going to write. + const base = { name: 'merge', fileName: basename(command.merged), content } + const show = (extra) => { + // Nothing to decide is not the same as "resolved": a file with no markers is + // one this tool has no business rewriting. + if (!hasConflictMarkers(content)) return refuse('cliErrors.merge-no-conflicts') + beginMerge(command) + return mergeInputsFor(command.merged).then((inputs) => + deliver({ ...base, ...inputs, ...extra }) + ) + } + walkFor(command.merged) + .then((walk) => (walk.done ? releaseLauncher(command.merged, 'written') : show(walkOf(walk)))) + // Without this the renderer is never told, no sentinel is written, and the + // terminal that ran `git mergetool` waits forever on a decision nobody was + // asked for. The markers alone are enough to resolve it. + .catch(() => show({})) +} + +function releaseCurrentLaunch() { + const pending = mergeInProgress() + if (!pending) return + endMerge() + releaseLauncher(pending.merged, 'written') +} + +// Save targets the row the view has open. A row opened OUT OF ORDER is written +// and marked, and git's later launch for it short-circuits in walkFor. +// +// The launch's own file goes through the launch's fence instead, even when the +// list is what opened it: `writeConflictText` writes the same bytes to the same +// path but cannot release the launcher, so routing on `openRel` alone left the +// terminal waiting out its full two hours on a file already resolved. The list +// pre-selects that row, so this is the primary button's path. +function saveMerge(text) { + const open = openRel() + if (open && open !== launchRel) return writeConflictText(text) + clearOpenRel() + const res = writeMerged(text) + if (res.ok) noteResolved(launchRel) + return res +} + +export function registerMergeIpc() { + ipcMain.handle('merge:write', (e, text) => saveMerge(text)) + // Declining is an answer too: it releases the launcher and spends the session. + // Backing out of a row opened OUT OF ORDER is not that — it declines nothing, + // because git is still waiting on a different file. Cancelling there used to + // write `cancelled` for the launch's file, telling git a file the reader had + // not even looked at was refused. + ipcMain.handle('merge:cancel', () => { + const open = openRel() + clearOpenRel() + if (open && open !== launchRel) return { ok: true } + endConflictSession() + return cancelMerge() + }) + ipcMain.handle('merge:list', () => conflictRows()) + ipcMain.handle('merge:take', async (e, index, side) => { + const res = await takeSideAt({ index, side: side === 'theirs' ? 'theirs' : 'ours' }) + // Answering the row for the file THIS launch was given writes it directly, + // which leaves the launch's own session armed and the terminal waiting on a + // sentinel that would never come. Spend it here. + const wasCurrent = res.ok && res.rel === launchRel + if (wasCurrent) releaseCurrentLaunch() + return { ok: res.ok, error: res.error, wasCurrent } + }) + ipcMain.handle('merge:open', (e, index) => openConflictAt(index)) + ipcMain.handle('merge:endWalk', () => void endConflictSession()) +} diff --git a/src/main/mergeSession.js b/src/main/mergeSession.js index d4a4ac7..3e1fd04 100644 --- a/src/main/mergeSession.js +++ b/src/main/mergeSession.js @@ -38,6 +38,22 @@ export function walkReset() { */ export const doneSentinel = (merged) => `${merged}.diffbro-merge-done` +/** + * Release the launcher. Exported because a file the reader already answered + * OUT OF ORDER gets a launch of its own later, and that launch has no session + * to spend — it is answered before any window is shown. + * @param {'written'|'cancelled'} verdict + */ +export function releaseLauncher(merged, verdict) { + try { + writeFileSync(doneSentinel(merged), verdict, 'utf8') + return true + } catch { + // The launcher times out on its own; a sentinel it cannot read is not fatal. + return false + } +} + /** Remembered from the launch argv, never from a message. */ export function beginMerge({ merged, local, remote }) { pending = { merged, local, remote } @@ -57,11 +73,7 @@ export function cancelMerge() { if (!pending) return { ok: false } const { merged } = pending pending = null - try { - writeFileSync(doneSentinel(merged), 'cancelled', 'utf8') - } catch { - // The launcher times out on its own; a sentinel it cannot read is not fatal. - } + releaseLauncher(merged, 'cancelled') return { ok: true } } @@ -84,11 +96,8 @@ export function writeMerged(text) { } const path = pending.merged endMerge() - try { - writeFileSync(doneSentinel(path), 'written', 'utf8') - } catch { - // Falling back to the launcher's own timeout is better than failing a write - // that already landed. - } + // Falling back to the launcher's own timeout is better than failing a write + // that already landed. + releaseLauncher(path, 'written') return { ok: true, path } } diff --git a/src/preload/index.js b/src/preload/index.js index 9f4d5dc..193c8a2 100644 --- a/src/preload/index.js +++ b/src/preload/index.js @@ -6,6 +6,14 @@ contextBridge.exposeInMainWorld('api', { readFile: (path, opts) => ipcRenderer.invoke('file:read', path, opts), writeMerged: (text) => ipcRenderer.invoke('merge:write', text), cancelMerge: () => ipcRenderer.invoke('merge:cancel'), + // The conflicted files of the merge git is walking. A row is addressed by its + // INDEX into the list main built — the renderer never learns, or names, a + // path, which is what keeps the one write the app makes outside its own files + // fenced in main. + conflictList: () => ipcRenderer.invoke('merge:list'), + takeConflictSide: (index, side) => ipcRenderer.invoke('merge:take', index, side), + openConflict: (index) => ipcRenderer.invoke('merge:open', index), + endMergeWalk: () => ipcRenderer.invoke('merge:endWalk'), // `format` names a row of main's own export table; never an extension. exportDiffFile: (payload) => ipcRenderer.invoke('diff:exportFile', payload), // Streamed comparison: files too large to hold are indexed by line in main diff --git a/src/renderer/src/components/AppDialogs.vue b/src/renderer/src/components/AppDialogs.vue index fd0a5db..9de7df2 100644 --- a/src/renderer/src/components/AppDialogs.vue +++ b/src/renderer/src/components/AppDialogs.vue @@ -37,6 +37,7 @@ import { useShareStore } from '../features/share' import { EmailHandoffDialog, useEmailStore } from '../features/email' +import { ConflictsDialog, useConflictsStore } from '../features/merge' import { useUiStore } from '../stores/uiStore' const store = useDiffStore() @@ -51,6 +52,7 @@ const paste = usePasteToCompareStore() const snippets = useSnippetStore() const vault = useVaultStore() const errors = useErrorStore() +const conflicts = useConflictsStore() diff --git a/src/renderer/src/components/AppToolbar.vue b/src/renderer/src/components/AppToolbar.vue index e327f4f..5a98fa2 100644 --- a/src/renderer/src/components/AppToolbar.vue +++ b/src/renderer/src/components/AppToolbar.vue @@ -12,6 +12,8 @@ import { useDiffStore } from '../stores/diffStore' import { useUiStore } from '../stores/uiStore' import { useImageExportStore } from '../features/imageExport' import { ZOOM_DEFAULT, zoomLabel } from '../utils/diffZoom' +import { useConflictsStore } from '../features/merge' +import AppIcon from './AppIcon.vue' import KeyActions from './KeyActions.vue' import ToolbarOverflow from './ToolbarOverflow.vue' import ViewOptionsMenu from './ViewOptionsMenu.vue' @@ -19,6 +21,7 @@ import ViewOptionsMenu from './ViewOptionsMenu.vue' const store = useDiffStore() const ui = useUiStore() const imageExport = useImageExportStore() +const conflicts = useConflictsStore() // Only while the comparison is NOT at its resting size: an unzoomed diff has // nothing to say, and a permanent "100%" would spend width the bar does not have @@ -50,6 +53,18 @@ const actionState = computed(() => ({ the same words the diagram diff uses. -->
+ + - + @@ -82,7 +111,10 @@ useMergeKeys(go) {{ $t('merge.fromBranch', { name: merge.oursName }) }} -
+
+
+ +
@@ -90,7 +122,9 @@ useMergeKeys(go) {{ $t('merge.result') }} {{ $t('merge.editable') }} -
+
+
+
@@ -100,7 +134,10 @@ useMergeKeys(go) {{ $t('merge.fromBranch', { name: merge.theirsName }) }} -
+
+
+ +
diff --git a/src/renderer/src/features/merge/components/styles/ConflictRow.css b/src/renderer/src/features/merge/components/styles/ConflictRow.css new file mode 100644 index 0000000..de9f944 --- /dev/null +++ b/src/renderer/src/features/merge/components/styles/ConflictRow.css @@ -0,0 +1,82 @@ +/* A row of the conflicts list. Sits on the well's --bg, so the state inks are + the --mg-* roles, which clear 4.5:1 there on all 20 themes. */ +.cf-row { + position: relative; + display: flex; + align-items: center; + gap: var(--space-2); + padding: 0 var(--space-3) 0 calc(var(--space-3) + 3px); + min-height: var(--control-h); + font-size: var(--font-md); + cursor: pointer; +} +.cf-stripe { + position: absolute; + inset-block: 0; + inset-inline-start: 0; + width: 3px; +} +/* One ink per state, carried on the row so the stripe fills with it and the + icon and word inherit it. */ +.cf-row.open { + --cf-ink: var(--mg-open); +} +.cf-row.done { + --cf-ink: var(--mg-done); +} +.cf-row.blocked { + --cf-ink: var(--mg-blocked); +} +.cf-stripe { + background: var(--cf-ink); +} +.cf-icon, +.cf-state { + color: var(--cf-ink); +} +.cf-icon { + flex: none; + font-size: var(--font-md); +} +.cf-name { + font-family: var(--font-mono); + color: var(--text); + white-space: nowrap; +} +/* --text-hint, not --text-dim: dim scores 2.96 on a hovered row (sepia), under + the reading floor. The directory is also what gives way first. */ +.cf-dir { + flex: 1; + min-width: 0; + font-size: var(--font-sm); + color: var(--text-hint); + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +} +.cf-state { + font-size: var(--font-sm); + white-space: nowrap; +} +/* Quiet until wanted, but NEVER `visibility: hidden` — that takes the buttons + out of the tab order, so the :focus-within below could not fire and the only + way to answer a row was the mouse. */ +.cf-take { + display: inline-flex; + gap: var(--space-1); + opacity: 0; +} +.cf-row:hover .cf-take, +.cf-row:focus-within .cf-take, +.cf-row.selected .cf-take { + opacity: 1; +} +.cf-row:hover { + background: var(--bg-hover); +} +/* Selection is never the hover wash alone — it scores 1.15 against --bg on + amber, which is no mark at all. The keyline is what makes it one. */ +.cf-row.selected { + background: var(--bg-hover); + box-shadow: inset 0 0 0 1px var(--btn-edge); +} diff --git a/src/renderer/src/features/merge/components/styles/ConflictsDialog.css b/src/renderer/src/features/merge/components/styles/ConflictsDialog.css new file mode 100644 index 0000000..e7b52a0 --- /dev/null +++ b/src/renderer/src/features/merge/components/styles/ConflictsDialog.css @@ -0,0 +1,33 @@ +/* The list is a reading surface inside the dialog's chrome, so it is --bg (the + base surface) within the --bg-panel dialog — the same relationship an input + has to the panel around it. */ +.cf-band { + justify-content: space-between; + gap: var(--space-2); + min-height: var(--chip-h); + font-size: var(--font-sm); + color: var(--text-hint); +} +.cf-tally b { + color: var(--text); + font-weight: 600; +} +.cf-revs { + font-family: var(--font-mono); + font-size: var(--font-xs); + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +} +.cf-well { + background: var(--bg); + border: 1px solid var(--border); + border-radius: var(--radius); + overflow: hidden auto; + max-height: 52vh; +} +.cf-hint { + flex: 1; + font-size: var(--font-xs); + color: var(--text-hint); +} diff --git a/src/renderer/src/features/merge/components/styles/MergeTakeOverlay.css b/src/renderer/src/features/merge/components/styles/MergeTakeOverlay.css new file mode 100644 index 0000000..38311fc --- /dev/null +++ b/src/renderer/src/features/merge/components/styles/MergeTakeOverlay.css @@ -0,0 +1,58 @@ +/* Buttons over the editor, straddling the divider on the side that faces the + result. The layer takes no pointer events so the editor beneath keeps its + selection and caret; only the buttons do. */ +.merge-takes { + position: absolute; + inset-block: 0; + width: 0; + pointer-events: none; + z-index: 2; +} +.merge-takes.ours { + inset-inline-end: 0; +} +.merge-takes.theirs { + inset-inline-start: 0; +} +.merge-take-btn { + position: absolute; + width: var(--chip-h); + height: var(--chip-h); + padding: 0; + display: inline-flex; + align-items: center; + justify-content: center; + border-radius: var(--radius-pill); + background: var(--bg); + /* --btn-edge-strong, not --btn-edge: this floats on a tinted conflict band, + where the ordinary keyline lands at 2.99 (sepia) and 3.00 (meridian). */ + border: 1px solid var(--btn-edge-strong); + box-shadow: var(--shadow-1); + cursor: pointer; + pointer-events: auto; + font-size: var(--font-sm); +} +.merge-takes.ours .merge-take-btn { + inset-inline-end: calc(var(--chip-h) / -2); + color: var(--dg-del); +} +.merge-takes.theirs .merge-take-btn { + inset-inline-start: calc(var(--chip-h) / -2); + color: var(--dg-add); +} +.merge-take-btn:hover { + background: var(--bg-hover); +} +.merge-take-btn:focus-visible { + outline: 2px solid var(--focus-ring); + outline-offset: 1px; +} +/* A region the reader is not on stays drawn but recedes, so the file's shape is + visible without every button competing for the eye. */ +.merge-take-btn.dim { + opacity: 0.45; +} +.merge-take-btn.dim:hover, +.merge-take-btn.dim:focus-visible { + opacity: 1; +} diff --git a/src/renderer/src/features/merge/components/styles/MergeView.css b/src/renderer/src/features/merge/components/styles/MergeView.css index bb02d2d..d1c0e4b 100644 --- a/src/renderer/src/features/merge/components/styles/MergeView.css +++ b/src/renderer/src/features/merge/components/styles/MergeView.css @@ -21,18 +21,37 @@ color: var(--text); white-space: nowrap; } -/* Height from --chip-h, never from padding. */ +/* Height from --chip-h, never from padding. It reads as a chip and behaves as a + button: this is the way back to the conflicts list once it is dismissed. */ .merge-walk { height: var(--chip-h); padding: 0 var(--space-2); display: inline-flex; align-items: center; + gap: var(--space-1); + border: 1px solid var(--btn-edge); border-radius: var(--radius-pill); background: var(--btn-face); color: var(--text); + font: inherit; font-size: var(--font-xs); font-variant-numeric: tabular-nums; white-space: nowrap; + cursor: pointer; +} +.merge-walk:hover:not(:disabled) { + background: var(--btn-face-hover); +} +.merge-walk:focus-visible { + outline: 2px solid var(--focus-ring); + outline-offset: 1px; +} +/* No live walk to go back to — flat, not the same control at a lower opacity. */ +.merge-walk:disabled { + border-color: var(--border); + background: none; + color: var(--text-hint); + cursor: default; } .merge-actions { display: flex; @@ -137,7 +156,14 @@ overflow: hidden; text-overflow: ellipsis; } -.merge-editor { +/* Positioning context for the take buttons, which sit ON the divider — so the + overflow stays visible where a pane would otherwise clip them. */ +.merge-editor-area { + position: relative; flex: 1; min-height: 0; } +.merge-editor { + position: absolute; + inset: 0; +} diff --git a/src/renderer/src/features/merge/conflictsStore.js b/src/renderer/src/features/merge/conflictsStore.js new file mode 100644 index 0000000..a4fc865 --- /dev/null +++ b/src/renderer/src/features/merge/conflictsStore.js @@ -0,0 +1,110 @@ +import { defineStore } from 'pinia' + +// The list of files git is still calling conflicted, and which of them this +// session has answered. Rows arrive from main and carry NO path — a row is +// acted on by its index, which main resolves back to a file. + +const rowsOf = (payload) => (Array.isArray(payload?.conflicts) ? payload.conflicts : []) + +export const useConflictsStore = defineStore('conflicts', { + state: () => ({ + /** @type {Array<{name: string, dir: string, tally: number, state: string}>} */ + rows: [], + /** Which row the reader is on — the one Resolve opens. */ + at: 0, + /** The dialog itself. Closing it never ends the merge; see `live`. */ + open: false, + /** A mergetool run is under way, so the way back in is offered. */ + live: false, + /** + * The reader closed the list. git launches once per file, so without this + * a thirty-file walk put the same modal in front of them thirty times. + */ + dismissed: false, + busy: false, + error: '' + }), + getters: { + remaining: (s) => s.rows.filter((r) => r.state !== 'done').length, + done: (s) => s.rows.filter((r) => r.state === 'done').length, + // One file is not a list, and a dialog to choose from it is in the way. + isWalk: (s) => s.rows.length > 1, + current: (s) => s.rows[s.at] ?? null + }, + actions: { + /** @param {object} payload the mergetool command main delivered */ + begin(payload) { + this.rows = rowsOf(payload) + this.at = Math.max(0, Number(payload?.at) || 0) + this.live = true + this.error = '' + this.reveal() + }, + select(index) { + if (index >= 0 && index < this.rows.length) this.at = index + }, + /** Offer the list unless the reader has already put it away. */ + reveal() { + if (this.isWalk && !this.dismissed) this.open = true + }, + /** Asked for by name — a chip, a menu item — so it overrides the dismissal. */ + show() { + this.dismissed = false + this.reveal() + }, + /** Close it to get at something behind it — stepping into a row. */ + hide() { + this.open = false + }, + /** The reader put it away, so the next launch does not push it back. */ + dismiss() { + this.open = false + this.dismissed = true + }, + // The walk is over: git has stopped asking, so the way back in goes away + // rather than offering to reconstruct a session that no longer exists. It + // also drops main's list, which is what closes the write handlers. + async end() { + this.open = false + this.live = false + this.dismissed = false + this.rows = [] + await window.api.endMergeWalk?.() + }, + async refresh() { + const rows = await window.api.conflictList() + if (Array.isArray(rows)) this.rows = rows + }, + /** + * Answer a whole file from the index, without opening it. + * @returns {Promise} whether the row answered was the one the view + * currently has open — which main has just released, so the view must go. + */ + async take(index, side) { + if (this.busy) return false + this.busy = true + const res = await window.api.takeConflictSide(index, side) + this.busy = false + this.error = res?.ok ? '' : res?.error === 'gone' ? 'gone' : 'take-failed' + await this.refresh() + return !!res?.wasCurrent + }, + /** + * The three-way payload for a row, so the view can open a file this launch + * was not given. Null when the row cannot be read as text. + */ + async payloadAt(index) { + const res = await window.api.openConflict(index) + if (!res?.ok) { + this.error = 'blocked' + return null + } + this.error = '' + return res + }, + markDone(index) { + const row = this.rows[index] + if (row) Object.assign(row, { state: 'done', tally: 0 }) + } + } +}) diff --git a/src/renderer/src/features/merge/index.js b/src/renderer/src/features/merge/index.js index 3cebed8..c06fbfb 100644 --- a/src/renderer/src/features/merge/index.js +++ b/src/renderer/src/features/merge/index.js @@ -1,2 +1,4 @@ export { useMergeStore } from './mergeStore' +export { useConflictsStore } from './conflictsStore' export { default as MergeView } from './components/MergeView.vue' +export { default as ConflictsDialog } from './components/ConflictsDialog.vue' diff --git a/src/renderer/src/features/merge/mergeDecorations.js b/src/renderer/src/features/merge/mergeDecorations.js index 9fe72f2..ef42fd6 100644 --- a/src/renderer/src/features/merge/mergeDecorations.js +++ b/src/renderer/src/features/merge/mergeDecorations.js @@ -1,10 +1,9 @@ -// What the three panes draw: the region bands, the gutter chevrons, the marks -// in the scrollbar that say where the rest of the conflicts are, and the -// intra-line tint that narrows a whole band down to the words that differ. +// What the three panes draw: the region bands, the marks in the scrollbar that +// say where the rest of the conflicts are, and the intra-line tint that narrows +// a whole band down to the words that differ. // // Pure builders — they take positions and colours and give back decorations. import * as monaco from 'monaco-editor' -import { t } from '../../i18n' import { wordSpans } from './mergeWords' const FULL = monaco.editor.OverviewRulerLane.Full @@ -48,15 +47,14 @@ export const regionOptions = (region, isCurrent, colors) => ({ // Each side keeps its own colour rather than borrowing the middle's: three tints // of the same one would have to be told apart, and the middle's is about state. +// +// No glyph-margin chevron: the take control is a real button on the pane's +// INNER edge (MergeTakeOverlay), because a glyph margin only exists on an +// editor's left edge and is not reachable from the keyboard. const bandOptions = (key, colors) => ({ isWholeLine: true, className: `merge-side-${key}`, linesDecorationsClassName: `merge-edge-${key}`, - // Pointing INWARD, so the direction of the chevron is the direction of the move. - glyphMarginClassName: `merge-take merge-take-${key}`, - glyphMarginHoverMessage: { - value: t(key === 'ours' ? 'merge.takeOursTip' : 'merge.takeTheirsTip') - }, overviewRuler: ruler(colors[key]) }) diff --git a/src/renderer/src/features/merge/mergePaneOps.js b/src/renderer/src/features/merge/mergePaneOps.js index 51ada22..ca26056 100644 --- a/src/renderer/src/features/merge/mergePaneOps.js +++ b/src/renderer/src/features/merge/mergePaneOps.js @@ -6,6 +6,7 @@ import * as monaco from 'monaco-editor' import { gutterAnchors } from './mergeGutter' import { applyChoice, touchedIndexes, wholeLines } from './mergeEdits' import { regionOptions, rulerColors, sideDecorations } from './mergeDecorations' +import { takeButtons } from './mergeTakeOverlay' export const SIDES = ['ours', 'result', 'theirs'] @@ -139,12 +140,21 @@ function reanchor({ editors, merge, ids, index, start, written }) { )[0] } -// A click in a side's glyph margin answers the region that sits there. -export function takeFromGutter({ anchors, key, event, take }) { - if (event.target?.type !== monaco.editor.MouseTargetType.GUTTER_GLYPH_MARGIN) return - const line = event.target.position?.lineNumber - const index = anchors[key].findIndex((a) => a && line >= a.line && line <= a.line + a.count - 1) - if (index >= 0) take(index, key) +/** + * Where each side's take buttons go, read off the live editor. Called on every + * repaint and every scroll — the geometry is Monaco's, the arithmetic is + * mergeTakeOverlay's. + */ +export function takeLayout({ editors, anchors, merge, key }) { + const editor = editors[key] + if (!editor?.getTopForLineNumber) return [] + return takeButtons({ + anchors: anchors[key], + topOf: (line) => editor.getTopForLineNumber(line), + scrollTop: editor.getScrollTop(), + height: editor.getLayoutInfo?.().height ?? 0, + current: merge.at + }) } /** Bring a region into view and put the caret on it. */ diff --git a/src/renderer/src/features/merge/mergeStore.js b/src/renderer/src/features/merge/mergeStore.js index 3fa2f5d..11af55c 100644 --- a/src/renderer/src/features/merge/mergeStore.js +++ b/src/renderer/src/features/merge/mergeStore.js @@ -96,6 +96,13 @@ export const useMergeStore = defineStore('merge', { async close() { this.open = false if (!this.saved) await window.api.cancelMerge() + }, + // The file was answered elsewhere — from its row in the conflicts list — + // so main has already written it and released the launcher. Telling main + // again would cancel a session that no longer exists. + dismiss() { + this.saved = true + this.open = false } } }) diff --git a/src/renderer/src/features/merge/mergeTakeOverlay.js b/src/renderer/src/features/merge/mergeTakeOverlay.js new file mode 100644 index 0000000..d97120f --- /dev/null +++ b/src/renderer/src/features/merge/mergeTakeOverlay.js @@ -0,0 +1,31 @@ +// Where a side's take buttons sit: one per region that side contains, at the top +// of its band, on the pane's INNER edge — against the result it moves text into. +// Monaco's glyph margin, which this replaces, only exists on an editor's LEFT +// edge and cannot be reached from the keyboard at all. + +// Half the button — --chip-h in tokens.css — so one straddling the top edge +// still reads as clickable. +const BLEED = 10 + +/** + * @param {object} view + * @param {Array<{line: number, count: number}|null>} view.anchors from mergeGutter + * @param {(line: number) => number} view.topOf the editor's own line geometry + * @param {number} view.scrollTop + * @param {number} view.height the pane's visible height + * @param {number} view.current the region the reader is on + * @returns {Array<{index: number, top: number, dim: boolean}>} + */ +export function takeButtons({ anchors, topOf, scrollTop, height, current }) { + const out = [] + ;(anchors ?? []).forEach((anchor, index) => { + // A side that deleted these lines has no band here and nothing to take. + if (!anchor) return + const top = Math.round(topOf(anchor.line) - scrollTop) + // Outside the viewport it would still be in the DOM and still focusable, + // which is how an overlay button ends up floating over a header. + if (top < -BLEED || top > height) return + out.push({ index, top, dim: index !== current }) + }) + return out +} diff --git a/src/renderer/src/features/merge/useMergePanes.js b/src/renderer/src/features/merge/useMergePanes.js index 602e547..78d358c 100644 --- a/src/renderer/src/features/merge/useMergePanes.js +++ b/src/renderer/src/features/merge/useMergePanes.js @@ -1,4 +1,4 @@ -import { onBeforeUnmount, watch } from 'vue' +import { onBeforeUnmount, ref, watch } from 'vue' import * as monaco from 'monaco-editor' import { applyMonacoTheme, MONACO_THEME } from '../../composables/useMonacoTheme' import { useSettingsStore } from '../../stores/settingsStore' @@ -10,7 +10,7 @@ import { seedRegions, SIDES, syncScroll, - takeFromGutter, + takeLayout, writeChoice } from './mergePaneOps' @@ -24,7 +24,6 @@ const BASE_OPTIONS = { scrollBeyondLastLine: false, contextmenu: false, fontSize: 12.5, - glyphMargin: true, // No completions while resolving a merge: the suggest widget offers words from // the file itself, which is noise here — and it swallows the Enter that was // meant to be a newline. @@ -38,7 +37,7 @@ const BASE_OPTIONS = { const makeEditor = (el, value, readOnly) => monaco.editor.create(el, { ...BASE_OPTIONS, value, readOnly }) -function build({ containers, merge, editors, sync, onGlyph, onEdit }) { +function build({ containers, merge, editors, sync, onScroll, onEdit }) { for (const side of SIDES) { const isResult = side === 'result' editors[side] = makeEditor( @@ -46,8 +45,10 @@ function build({ containers, merge, editors, sync, onGlyph, onEdit }) { isResult ? merge.result : merge[side], !isResult ) - editors[side].onDidScrollChange(() => syncScroll(editors, side, sync)) - if (!isResult) editors[side].onMouseDown((e) => onGlyph(side, e)) + editors[side].onDidScrollChange(() => { + syncScroll(editors, side, sync) + onScroll() + }) } // The reader typing IS the answer where neither side got it right. editors.result.onDidChangeModelContent(() => { @@ -73,18 +74,31 @@ export function useMergePanes(containers, merge) { const sync = { busy: false } const anchors = { ours: [], theirs: [] } const settled = [] + // The take buttons are DOM over the editor, so their positions are state the + // template renders rather than decorations Monaco keeps. One ref, so the + // template unwraps it. + const takes = ref({ ours: [], theirs: [] }) let ids = {} function create() { if (editors.result || !containers.result.value) return applyMonacoTheme(settings.theme) - build({ containers, merge, editors, sync, onGlyph, onEdit }) + build({ containers, merge, editors, sync, onScroll: place, onEdit }) seedRegions({ editors, merge, ids, settled }) paint() } - const paint = () => repaint({ editors, merge, ids, anchors }) - const onGlyph = (key, event) => takeFromGutter({ anchors, key, event, take }) + const place = () => { + takes.value = { + ours: takeLayout({ editors, anchors, merge, key: 'ours' }), + theirs: takeLayout({ editors, anchors, merge, key: 'theirs' }) + } + } + + const paint = () => { + repaint({ editors, merge, ids, anchors }) + place() + } const onEdit = () => resolveTouched({ editors, merge, ids, settled }) const reveal = (index) => revealRegion({ editors, ids, index }) @@ -107,7 +121,7 @@ export function useMergePanes(containers, merge) { bind({ containers, merge, settings, editors, paint, create, destroy }) onBeforeUnmount(destroy) - return { create, reveal, take, takeAll } + return { create, reveal, take, takeAll, takes } } // The watchers, out of the factory so it stays inside the size cap. diff --git a/src/renderer/src/menus.js b/src/renderer/src/menus.js index 0537cac..490c68b 100644 --- a/src/renderer/src/menus.js +++ b/src/renderer/src/menus.js @@ -30,6 +30,65 @@ const displayToggles = (run) => [ // Mirrors the native Terminal menu (menuSections.terminalMenu). const terminalItems = (run) => [{ label: t('menu.terminal.info'), run: () => run('settings-cli') }] +const viewSection = (run) => ({ + id: 'view', + label: t('menu.view.title'), + items: [ + { + label: t('menu.view.commandPalette'), + keys: `${MOD}+Shift+P`, + paletteHidden: true, + run: () => run('command-palette') + }, + { sep: true }, + ...displayToggles(run), + { + label: t('menu.view.toggleSidebar'), + keys: `${MOD}+B`, + run: () => run('toggle-sidebar') + }, + { label: t('menu.view.toggleTheme'), keys: `${MOD}+D`, run: () => run('toggle-theme') }, + { sep: true }, + { + // No key hint: the binding is user-configurable (Settings → + // Shortcuts), so a fixed label here would go stale once rebound. + label: t('menu.view.quickLook'), + run: () => window.api.quickLookToggle() + }, + { + label: t('menu.view.mergeConflicts'), + keys: `${MOD}+Shift+K`, + run: () => run('merge-conflicts') + }, + { sep: true }, + { + label: t('menu.view.zoomIn'), + keys: `${MOD}++`, + paletteHidden: true, + run: () => run('zoom-in') + }, + { + label: t('menu.view.zoomOut'), + keys: `${MOD}+-`, + paletteHidden: true, + run: () => run('zoom-out') + }, + { + label: t('menu.view.resetZoom'), + keys: `${MOD}+0`, + paletteHidden: true, + run: () => run('zoom-reset') + }, + { sep: true, devOnly: true }, + { + label: t('menu.view.devTools'), + devOnly: true, + paletteHidden: true, + run: () => window.api.toggleDevTools() + } + ] +}) + export function buildMenus(run) { return [ { @@ -93,59 +152,7 @@ export function buildMenus(run) { { label: t('menu.edit.pasteTextMode'), keys: `${MOD}+T`, run: () => run('toggle-paste') } ] }, - { - id: 'view', - label: t('menu.view.title'), - items: [ - { - label: t('menu.view.commandPalette'), - keys: `${MOD}+Shift+P`, - paletteHidden: true, - run: () => run('command-palette') - }, - { sep: true }, - ...displayToggles(run), - { - label: t('menu.view.toggleSidebar'), - keys: `${MOD}+B`, - run: () => run('toggle-sidebar') - }, - { label: t('menu.view.toggleTheme'), keys: `${MOD}+D`, run: () => run('toggle-theme') }, - { sep: true }, - { - // No key hint: the binding is user-configurable (Settings → - // Shortcuts), so a fixed label here would go stale once rebound. - label: t('menu.view.quickLook'), - run: () => window.api.quickLookToggle() - }, - { sep: true }, - { - label: t('menu.view.zoomIn'), - keys: `${MOD}++`, - paletteHidden: true, - run: () => run('zoom-in') - }, - { - label: t('menu.view.zoomOut'), - keys: `${MOD}+-`, - paletteHidden: true, - run: () => run('zoom-out') - }, - { - label: t('menu.view.resetZoom'), - keys: `${MOD}+0`, - paletteHidden: true, - run: () => run('zoom-reset') - }, - { sep: true, devOnly: true }, - { - label: t('menu.view.devTools'), - devOnly: true, - paletteHidden: true, - run: () => window.api.toggleDevTools() - } - ] - }, + viewSection(run), { id: 'terminal', label: t('menu.terminal.title'), items: terminalItems(run) }, { id: 'security', label: t('menu.security.title'), items: securityItems(run) }, { diff --git a/src/renderer/src/styles/tokens.css b/src/renderer/src/styles/tokens.css index 7f01c6f..28a6bd9 100644 --- a/src/renderer/src/styles/tokens.css +++ b/src/renderer/src/styles/tokens.css @@ -89,6 +89,12 @@ 1.24 on bloom — no edge at all; halfway to --text clears the 3:1 non-text floor on all 14 (weakest sepia 3.19). */ --btn-edge: color-mix(in srgb, var(--border) 50%, var(--text)); + /* A control that floats ON a tinted band rather than on a surface — the merge + view's take buttons, which sit over a conflict band. The band closes the + gap --btn-edge was tuned for: it lands at 2.99 on sepia and exactly 3.00 on + meridian, at or under the 3:1 non-text floor. A quarter of the way from the + border to the ink clears every band tint on all 20 (worst 4.88, sepia). */ + --btn-edge-strong: color-mix(in srgb, var(--border) 25%, var(--text)); /* The primary button's hover/press ladder. It steps the KEYLINE, never the fill: moving a filled button's background toward the label printed on it can only close the gap between them, which is how .btn-primary:hover came @@ -144,4 +150,16 @@ --dg-add: var(--success-text); --dg-del: color-mix(in srgb, var(--danger-border) 70%, var(--text)); --dg-chg: var(--warning-border); + + /* Merge-conflict status, as INK on a row of the conflicts list. The --dg-* + roles above are tuned for the 3:1 non-text floor — a band, a stroke, a + badge — and a row's state is a WORD, so it answers to 4.5 instead. Raw, + --warning-border scores 2.73 on light and 2.74 on contrast; each is mixed + toward the theme's own ink until the worst of the twenty clears the floor + (open 4.67, done 4.59, blocked 4.60 — sepia, sepia, nord). The percentages + differ because the three source roles do not start from the same place: + --danger-border is the weakest, so its mix stops at 35%. */ + --mg-open: color-mix(in srgb, var(--warning-border) 45%, var(--text)); + --mg-done: color-mix(in srgb, var(--success-text) 55%, var(--text)); + --mg-blocked: color-mix(in srgb, var(--danger-border) 35%, var(--text)); } diff --git a/src/renderer/src/styles/ui.css b/src/renderer/src/styles/ui.css index 2f95f80..b96aec8 100644 --- a/src/renderer/src/styles/ui.css +++ b/src/renderer/src/styles/ui.css @@ -1119,29 +1119,3 @@ button.tag-chip:hover .usb-tct { box-shadow: inset 3px 0 0 color-mix(in srgb, var(--dg-add) 60%, transparent); } -/* The chevron in a side pane's glyph margin: click it and that side moves into - the result. An SVG mask, never a glyph character — a » or « tofus on any font - that lacks it, which is the rule icons.js exists to enforce. The mask takes - its colour from the side it belongs to. */ -.merge-take { - cursor: pointer; - -webkit-mask-repeat: no-repeat; - -webkit-mask-position: center; - -webkit-mask-size: 13px 13px; - mask-repeat: no-repeat; - mask-position: center; - mask-size: 13px 13px; -} -.merge-take:hover { - background-color: var(--accent); -} -.merge-take-ours { - background-color: var(--dg-del); - -webkit-mask-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24' fill='none' stroke='%23000' stroke-width='2.5' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='m9 18 6-6-6-6'/%3E%3C/svg%3E"); - mask-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24' fill='none' stroke='%23000' stroke-width='2.5' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='m9 18 6-6-6-6'/%3E%3C/svg%3E"); -} -.merge-take-theirs { - background-color: var(--dg-add); - -webkit-mask-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24' fill='none' stroke='%23000' stroke-width='2.5' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='m15 18-6-6 6-6'/%3E%3C/svg%3E"); - mask-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24' fill='none' stroke='%23000' stroke-width='2.5' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='m15 18-6-6 6-6'/%3E%3C/svg%3E"); -} diff --git a/src/renderer/src/types.js b/src/renderer/src/types.js index ee4a593..14d3203 100644 --- a/src/renderer/src/types.js +++ b/src/renderer/src/types.js @@ -380,4 +380,16 @@ * @property {() => void} [open] expand a collapsed section */ +/** + * One row of the merge-conflicts list, as main hands it over. It carries NO + * path: a row is acted on by its index into the list main built. + * @typedef {object} ConflictRowData + * @property {string} name the file's own name + * @property {string} dir repo-relative directory, empty at the root + * @property {number} tally conflict regions still in the working copy + * @property {'open'|'done'|'blocked'} state + * 'blocked' is binary, oversized, unreadable or marker-free — answerable + * whole-file from the index, never opened as text + */ + export {} diff --git a/src/renderer/src/utils/commands.js b/src/renderer/src/utils/commands.js index ac22321..8690eb9 100644 --- a/src/renderer/src/utils/commands.js +++ b/src/renderer/src/utils/commands.js @@ -18,6 +18,7 @@ import { tabsFullMessage } from './cliCommand' * @property {object} [imageExport] * @property {object} [configBackup] * @property {object} [share] + * @property {object} [conflicts] the merge walk's file list */ const openTool = (tool) => (s) => (s.ui.textTool = tool) @@ -44,6 +45,12 @@ export const COMMANDS = { 'tab-prev': ({ tabs }) => tabs.step(-1), 'tab-close': ({ tabs }) => tabs.requestActiveClose(), 'import-snippets': ({ diff }) => diff.importSnippets(), + // Reopening the list is the way back after dismissing it. Guarded on `live`: + // once git stops walking there is no session to show, and offering one would + // mean reconstructing a merge git has finished with. + 'merge-conflicts': ({ conflicts }) => { + if (conflicts?.live) conflicts.show() + }, // Hiding the toolbar button was not enough: the shortcut, the menu item and // the palette all reach the same action, and on a saved diff it would replace // what the reader opened. The guard belongs HERE, where every surface meets. @@ -142,7 +149,10 @@ export const CLI_COMMANDS = { // Typed in the terminal, so it is saved outright rather than opened in the // editor — the reader has already answered every question the editor asks. 'new-snippet': ({ snippets }, command) => snippets.add(command.draft), - merge: ({ merge }, command) => merge.begin(command), + merge: ({ merge, conflicts }, command) => { + conflicts.begin(command) + merge.begin(command) + }, compare: ({ diff, tabs }, command) => compareFromCli({ diff, tabs }, command.files, command.transient === true), // The passphrase is asked for here, not in the terminal: the bundle is diff --git a/src/renderer/src/utils/shortcuts.js b/src/renderer/src/utils/shortcuts.js index 189ee39..4ef59e0 100644 --- a/src/renderer/src/utils/shortcuts.js +++ b/src/renderer/src/utils/shortcuts.js @@ -53,6 +53,7 @@ export const SHORTCUT_GROUPS = [ { keys: `${MOD}+\\`, labelKey: 'shortcuts.toggleSplit' }, { keys: `${MOD}+B`, labelKey: 'shortcuts.toggleSidebar' }, { keys: `${MOD}+Shift+D`, labelKey: 'shortcuts.toggleStructure' }, + { keys: `${MOD}+Shift+K`, labelKey: 'shortcuts.mergeConflicts' }, { keys: `${MOD}+D`, labelKey: 'shortcuts.toggleTheme' }, { keys: `${MOD}++`, labelKey: 'shortcuts.zoomIn' }, { keys: `${MOD}+-`, labelKey: 'shortcuts.zoomOut' }, diff --git a/src/shared/i18n/en-XA.json b/src/shared/i18n/en-XA.json index d3779a8..f4180a8 100644 --- a/src/shared/i18n/en-XA.json +++ b/src/shared/i18n/en-XA.json @@ -36,7 +36,23 @@ "prevTip": "[Ƥřéṽĩōūş çōńƒłĩçţ · Šĥĩƒţ+Ƒ7 ·øé·øé·øé]", "nextTip": "[Ńéẋţ çōńƒłĩçţ · Ƒ7 ·øé·øé]", "fromBranch": "[ƒřōɱ {name} ·øé·]", - "mixedEndings": "[Ţĥĩş ƒĩłé ɱĩẋéş ÇŘŁƑ àńđ ŁƑ łĩńé éńđĩńğş. Šàṽĩńğ ŵřĩţéş éṽéřŷ łĩńé ŵĩţĥ ţĥé şàɱé ōńé. ·øé·øé·øé·øé·øé·øé·øé·øé·øé·]" + "mixedEndings": "[Ţĥĩş ƒĩłé ɱĩẋéş ÇŘŁƑ àńđ ŁƑ łĩńé éńđĩńğş. Šàṽĩńğ ŵřĩţéş éṽéřŷ łĩńé ŵĩţĥ ţĥé şàɱé ōńé. ·øé·øé·øé·øé·øé·øé·øé·øé·øé·]", + "conflictsTitle": "[Ṁéřğé çōńƒłĩçţş ·øé·øé]", + "fileCount": "[{n} ƒĩłé | {n} ƒĩłéş ·øé·ø]", + "filesResolved": "[{files} · {done} řéşōłṽéđ ·øé·øé·]", + "repoRoot": "[řéƥōşĩţōřŷ řōōţ ·øé·øé]", + "rowConflicts": "[{n} çōńƒłĩçţ | {n} çōńƒłĩçţş ·øé·øé·ø]", + "rowResolved": "[Řéşōłṽéđ ·øé·]", + "rowBlocked": "[Ɓĩńàřŷ — ƥĩçķ à şĩđé ·øé·øé]", + "resolveRow": "[Řéşōłṽé ·øé]", + "listHint": "[Éşç çłōşéş ţĥé łĩşţ — ţĥé ţōōłƀàř çĥĩƥ ƀřĩńğş ĩţ ƀàçķ ·øé·øé·øé·øé·øé·ø]", + "reopenTip": "[Šĥōŵ ţĥé çōńƒłĩçţéđ ƒĩłéş ·øé·øé·øé]", + "conflictsLeft": "[{n} çōńƒłĩçţ łéƒţ | {n} çōńƒłĩçţş łéƒţ ·øé·øé·øé·ø]", + "error": { + "gone": "[ğĩţ ńō łōńğéř çàłłş ţĥàţ ƒĩłé çōńƒłĩçţéđ, şō ńōţĥĩńğ ŵàş ŵřĩţţéń. ·øé·øé·øé·øé·øé·øé·øé·]", + "take-failed": "[Ţĥàţ şĩđé çōūłđ ńōţ ƀé řéàđ ōūţ ōƒ ţĥé ĩńđéẋ. ·øé·øé·øé·øé·ø]", + "blocked": "[Ţĥàţ ƒĩłé çàńńōţ ƀé ōƥéńéđ àş ţéẋţ. Àńşŵéř ĩţ ŵĩţĥ Ōūřş ōř Ţĥéĩřş. ·øé·øé·øé·øé·øé·øé·øé]" + } }, "menu": { "file": { @@ -73,6 +89,7 @@ "toggleSplit": "[Ţōğğłé Šƥłĩţ Ṽĩéŵ ·øé·øé]", "toggleTheme": "[Ţōğğłé Łĩğĥţ/Đàřķ Ţĥéɱé ·øé·øé·ø]", "quickLook": "[Ɋūĩçķ Łōōķ-ūƥ ·øé·ø]", + "mergeConflicts": "[Ṁéřğé çōńƒłĩçţş… ·øé·øé]", "zoomIn": "[Žōōɱ Ĩń ·øé]", "zoomOut": "[Žōōɱ Ōūţ ·øé]", "resetZoom": "[Řéşéţ Žōōɱ ·øé·]", @@ -204,6 +221,7 @@ "toggleSplit": "[Ţōğğłé şƥłĩţ ṽĩéŵ ·øé·øé]", "toggleSidebar": "[Ţōğğłé şĩđéƀàř ·øé·øé]", "toggleStructure": "[Ţōğğłé şţřūçţūřé ṽĩéŵ ·øé·øé·ø]", + "mergeConflicts": "[Ṁéřğé çōńƒłĩçţş ·øé·øé]", "toggleTheme": "[Ţōğğłé łĩğĥţ/đàřķ ţĥéɱé ·øé·øé·ø]", "zoomIn": "[Žōōɱ ţĥé đĩƒƒ ĩń ·øé·øé]", "zoomOut": "[Žōōɱ ţĥé đĩƒƒ ōūţ ·øé·øé]", diff --git a/src/shared/i18n/en.json b/src/shared/i18n/en.json index 03a08a8..13a6f89 100644 --- a/src/shared/i18n/en.json +++ b/src/shared/i18n/en.json @@ -36,7 +36,23 @@ "prevTip": "Previous conflict · Shift+F7", "nextTip": "Next conflict · F7", "fromBranch": "from {name}", - "mixedEndings": "This file mixes CRLF and LF line endings. Saving writes every line with the same one." + "mixedEndings": "This file mixes CRLF and LF line endings. Saving writes every line with the same one.", + "conflictsTitle": "Merge conflicts", + "fileCount": "{n} file | {n} files", + "filesResolved": "{files} · {done} resolved", + "repoRoot": "repository root", + "rowConflicts": "{n} conflict | {n} conflicts", + "rowResolved": "Resolved", + "rowBlocked": "Binary — pick a side", + "resolveRow": "Resolve", + "listHint": "Esc closes the list — the toolbar chip brings it back", + "reopenTip": "Show the conflicted files", + "conflictsLeft": "{n} conflict left | {n} conflicts left", + "error": { + "gone": "git no longer calls that file conflicted, so nothing was written.", + "take-failed": "That side could not be read out of the index.", + "blocked": "That file cannot be opened as text. Answer it with Ours or Theirs." + } }, "menu": { "file": { @@ -73,6 +89,7 @@ "toggleSplit": "Toggle Split View", "toggleTheme": "Toggle Light/Dark Theme", "quickLook": "Quick Look-up", + "mergeConflicts": "Merge conflicts…", "zoomIn": "Zoom In", "zoomOut": "Zoom Out", "resetZoom": "Reset Zoom", @@ -204,6 +221,7 @@ "toggleSplit": "Toggle split view", "toggleSidebar": "Toggle sidebar", "toggleStructure": "Toggle structure view", + "mergeConflicts": "Merge conflicts", "toggleTheme": "Toggle light/dark theme", "zoomIn": "Zoom the diff in", "zoomOut": "Zoom the diff out", diff --git a/tests/main/conflictList.test.js b/tests/main/conflictList.test.js new file mode 100644 index 0000000..9babc7a --- /dev/null +++ b/tests/main/conflictList.test.js @@ -0,0 +1,155 @@ +// The list git is walking, and every reason a row can refuse to be acted on. +// Pure core, so none of this needs a repository. +import { describe, expect, it } from 'vitest' +import { + conflictTally, + entryFor, + isRepoRelative, + pickEntry, + visibleEntries +} from '../../src/main/conflictList' + +const text = (s) => Buffer.from(s, 'utf8') + +const CONFLICTED = `head +<<<<<<< HEAD +ours +======= +theirs +>>>>>>> feature +middle +<<<<<<< HEAD +ours again +======= +theirs again +>>>>>>> feature +tail +` + +describe('conflictTally', () => { + it('counts the regions git opened', () => { + expect(conflictTally(CONFLICTED)).toBe(2) + }) + + it('is zero for a file with none', () => { + expect(conflictTally('nothing here\n')).toBe(0) + }) + + // The same seven-characters-then-space-or-EOL rule mergeGuards enforces: a + // line merely STARTING with them is ordinary text, and counting it would put + // a number on the row that the merge view then disagrees with. + it('ignores a line that merely starts with the marker characters', () => { + expect(conflictTally('<<<<<<<>>>>>> x\n')).toBe(1) + }) + + it('survives a CRLF file', () => { + expect(conflictTally('<<<<<<< HEAD\r\n=======\r\n>>>>>>> x\r\n')).toBe(1) + }) +}) + +describe('entryFor', () => { + it('describes a text conflict with its name, directory and tally', () => { + expect(entryFor({ rel: 'src/main/cliRoute.js', buffer: text(CONFLICTED) })).toEqual({ + rel: 'src/main/cliRoute.js', + name: 'cliRoute.js', + dir: 'src/main', + tally: 2, + state: 'open' + }) + }) + + // A file at the repository root has no directory to show; the renderer + // decides what to call that, because copy lives in the catalogue. + it('leaves the directory empty for a file at the root', () => { + expect(entryFor({ rel: 'package-lock.json', buffer: text(CONFLICTED) }).dir).toBe('') + }) + + // git calls the mergetool for a binary conflict too, and leaves no markers in + // it. Opening one as text would decode every invalid byte to U+FFFD. + it('blocks a binary file', () => { + const entry = entryFor({ + rel: 'resources/icon.png', + buffer: Buffer.from([0x89, 0x50, 0x00, 0x4e]) + }) + expect(entry.state).toBe('blocked') + expect(entry.tally).toBe(0) + }) + + it('blocks a file past the size ceiling rather than reading it', () => { + const entry = entryFor({ rel: 'huge.json', buffer: text('x'), size: 33 * 1024 * 1024 }) + expect(entry.state).toBe('blocked') + }) + + // Unreadable is not resolvable, but it is still a row: hiding it would make + // the tally disagree with what git says is left. + it('blocks a file it could not read', () => { + expect(entryFor({ rel: 'gone.js', buffer: null }).state).toBe('blocked') + }) +}) + +describe('visibleEntries', () => { + const paths = ['a.js', 'b.js'] + const read = (rel) => ({ buffer: rel === 'a.js' ? text(CONFLICTED) : text('plain\n') }) + + it('keeps git’s own order', () => { + expect(visibleEntries({ paths, read }).map((e) => e.name)).toEqual(['a.js', 'b.js']) + }) + + // A file with no markers left is one this tool has no business rewriting — + // the same refusal routeMerge already makes for a single file. + it('blocks a conflicted path whose working copy has no markers', () => { + expect(visibleEntries({ paths, read })[1].state).toBe('blocked') + }) + + it('marks the paths already resolved this session', () => { + const entries = visibleEntries({ paths, read, resolved: new Set(['b.js']) }) + expect(entries.map((e) => e.state)).toEqual(['open', 'done']) + }) +}) + +describe('pickEntry', () => { + const entries = [ + { rel: 'a.js', state: 'open' }, + { rel: 'b.js', state: 'done' } + ] + + it('answers the entry at that index', () => { + expect(pickEntry(entries, 0)).toEqual(entries[0]) + }) + + // The renderer sends an INDEX and nothing else, so every shape it could send + // has to fail closed rather than reach a path. + it('refuses an index that is not one of them', () => { + for (const bad of [-1, 2, 1.5, '0', null, undefined, NaN, Infinity]) { + expect(pickEntry(entries, bad)).toBe(null) + } + }) +}) + +describe('isRepoRelative', () => { + it('accepts a path inside the repository', () => { + expect(isRepoRelative('src/main/cliRoute.js')).toBe(true) + }) + + // git speaks posix paths in its own output, and a path that climbs out of the + // repository is not a path in it. These are the shapes that would turn an + // index into a write somewhere else. + it('refuses anything that leaves it, or that could read as an option', () => { + for (const bad of [ + '../outside.js', + 'src/../../etc/passwd', + '/etc/passwd', + '-oProxyCommand', + '', + null, + 42 + ]) { + expect(isRepoRelative(bad)).toBe(false) + } + }) + + it('refuses a windows separator climbing out', () => { + expect(isRepoRelative('src\\..\\..\\etc\\passwd')).toBe(false) + }) +}) diff --git a/tests/main/conflictSession.test.js b/tests/main/conflictSession.test.js new file mode 100644 index 0000000..6b01619 --- /dev/null +++ b/tests/main/conflictSession.test.js @@ -0,0 +1,242 @@ +// Custody of the conflict list, over real repositories — this module holds BOTH +// write paths and decides which file a Save lands on, and it is where two +// data-loss bugs lived: a row left armed from a previous repository, and a Save +// that wrote the right file but never released git. +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { execFileSync } from 'node:child_process' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { + conflictRows, + endConflictSession, + indexOfRel, + isResolvedRel, + locate, + openConflictAt, + openConflictSession, + openRel, + takeSideAt, + writeConflictText +} from '../../src/main/conflictSession' + +const dirs = [] +const GIT_ENV = { + GIT_AUTHOR_NAME: 'T', + GIT_AUTHOR_EMAIL: 't@e', + GIT_COMMITTER_NAME: 'T', + GIT_COMMITTER_EMAIL: 't@e' +} + +// Two conflicted files, so out-of-order answering has somewhere to go wrong. +function conflictedRepo(names = ['alpha.txt', 'beta.txt']) { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-session-')) + dirs.push(dir) + const git = (...args) => + execFileSync('git', args, { cwd: dir, env: { ...process.env, ...GIT_ENV } }) + const write = (name, body) => { + mkdirSync(dirname(join(dir, name)), { recursive: true }) + writeFileSync(join(dir, name), body) + } + git('init', '-q', '-b', 'main') + for (const name of names) write(name, 'top\nbase\nbottom\n') + git('add', '.') + git('commit', '-qm', 'base') + git('checkout', '-qb', 'feature') + for (const name of names) write(name, `top\ntheirs ${name}\nbottom\n`) + git('commit', '-qam', 'theirs') + git('checkout', '-q', 'main') + for (const name of names) write(name, `top\nours ${name}\nbottom\n`) + git('commit', '-qam', 'ours') + try { + git('merge', 'feature') + } catch { + // Expected: this is the conflict under test. + } + return { dir, git, path: (name) => join(dir, name) } +} + +beforeEach(() => endConflictSession()) + +afterEach(() => { + endConflictSession() + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }) +}) + +describe('locate', () => { + // The rel is what everything downstream is addressed by, and it must be git's + // own spelling: Windows hands out an 8.3 temp path where git answers with the + // long one, and deriving it from path.relative produced a `..` the fence then + // rejected — so the list never opened there at all. + it('finds the repository holding a mergetool’s file, and where it sits', async () => { + const repo = conflictedRepo() + const found = await locate(repo.path('alpha.txt')) + expect(found).toMatchObject({ rel: 'alpha.txt' }) + expect(found.root).toBeTruthy() + }) + + it('spells a file in a subdirectory the way git does', async () => { + const repo = conflictedRepo(['nested/deep/gamma.txt']) + const found = await locate(repo.path(join('nested', 'deep', 'gamma.txt'))) + expect(found).toMatchObject({ rel: 'nested/deep/gamma.txt' }) + }) + + it('answers null for a file outside any repository', async () => { + const loose = mkdtempSync(join(tmpdir(), 'diffbro-loose-')) + dirs.push(loose) + writeFileSync(join(loose, 'loose.txt'), 'x\n') + expect(await locate(join(loose, 'loose.txt'))).toBe(null) + }) +}) + +describe('the list', () => { + it('carries no path — only what a row displays', async () => { + const repo = conflictedRepo() + await openConflictSession(repo.dir) + const rows = conflictRows() + expect(rows).toHaveLength(2) + expect(Object.keys(rows[0]).sort()).toEqual(['dir', 'name', 'state', 'tally']) + expect(rows.map((r) => r.name)).toEqual(['alpha.txt', 'beta.txt']) + }) +}) + +describe('takeSideAt', () => { + it('writes a whole side out of the index and settles the row', async () => { + const repo = conflictedRepo() + await openConflictSession(repo.dir) + expect(await takeSideAt({ index: 1, side: 'theirs' })).toEqual({ ok: true, rel: 'beta.txt' }) + expect(readFileSync(repo.path('beta.txt'), 'utf8')).toBe('top\ntheirs beta.txt\nbottom\n') + expect(isResolvedRel('beta.txt')).toBe(true) + }) + + // The list can go stale between the dialog opening and the click. + it('refuses a row git no longer calls conflicted', async () => { + const repo = conflictedRepo() + await openConflictSession(repo.dir) + repo.git('add', 'beta.txt') + expect(await takeSideAt({ index: 1, side: 'ours' })).toEqual({ ok: false, error: 'gone' }) + }) + + it('refuses an index that is not a row', async () => { + const repo = conflictedRepo() + await openConflictSession(repo.dir) + for (const index of [-1, 99, 1.5, '0', null, undefined]) { + expect(await takeSideAt({ index, side: 'ours' })).toEqual({ ok: false, error: 'gone' }) + } + }) +}) + +describe('openConflictAt and the row a Save lands on', () => { + it('remembers which row is open, and gives the view its three sides', async () => { + const repo = conflictedRepo() + await openConflictSession(repo.dir) + const payload = await openConflictAt(1) + expect(payload).toMatchObject({ ok: true, fileName: 'beta.txt', position: 2, total: 2 }) + expect(payload.ours).toBe('top\nours beta.txt\nbottom\n') + expect(payload.theirs).toBe('top\ntheirs beta.txt\nbottom\n') + expect(openRel()).toBe('beta.txt') + }) + + it('writes the row that is open, not the one git asked about', async () => { + const repo = conflictedRepo() + await openConflictSession(repo.dir) + await openConflictAt(1) + expect(await writeConflictText('resolved beta\n')).toEqual({ ok: true }) + expect(readFileSync(repo.path('beta.txt'), 'utf8')).toBe('resolved beta\n') + expect(readFileSync(repo.path('alpha.txt'), 'utf8')).toContain('<<<<<<<') + }) + + // THE DATA-LOSS BUG. A mergetool launch for a file in no repository left the + // PREVIOUS repository's row armed, so the next Save wrote that repo's file + // with text the reader typed for a different one — silently, and destroying + // the conflict that was there. + it('forgets the open row when a launch has no repository behind it', async () => { + const repo = conflictedRepo() + await openConflictSession(repo.dir) + await openConflictAt(1) + endConflictSession() + expect(openRel()).toBe(null) + expect(await writeConflictText('text meant for somewhere else\n')).toEqual({ + ok: false, + error: 'no-merge' + }) + expect(readFileSync(repo.path('beta.txt'), 'utf8')).toContain('<<<<<<<') + }) + + // A second repository must not inherit the first one's open row either. + it('forgets the open row when the walk moves to another repository', async () => { + const first = conflictedRepo() + await openConflictSession(first.dir) + await openConflictAt(1) + const second = conflictedRepo(['gamma.txt']) + await openConflictSession(second.dir) + expect(openRel()).toBe(null) + expect(await writeConflictText('x\n')).toEqual({ ok: false, error: 'no-merge' }) + expect(readFileSync(first.path('beta.txt'), 'utf8')).toContain('<<<<<<<') + }) + + it('refuses to open a row that cannot be read as text', async () => { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-session-bin-')) + dirs.push(dir) + const git = (...a) => execFileSync('git', a, { cwd: dir, env: { ...process.env, ...GIT_ENV } }) + git('init', '-q', '-b', 'main') + writeFileSync(join(dir, 'blob.bin'), Buffer.from([0x01, 0x00, 0x02])) + git('add', '.') + git('commit', '-qm', 'base') + git('checkout', '-qb', 'feature') + writeFileSync(join(dir, 'blob.bin'), Buffer.from([0x03, 0x00, 0x04])) + git('commit', '-qam', 'theirs') + git('checkout', '-q', 'main') + writeFileSync(join(dir, 'blob.bin'), Buffer.from([0x05, 0x00, 0x06])) + git('commit', '-qam', 'ours') + try { + git('merge', 'feature') + } catch { + // Expected. + } + await openConflictSession(dir) + expect(conflictRows()[0].state).toBe('blocked') + expect(await openConflictAt(0)).toEqual({ ok: false, error: 'blocked' }) + }) + + // "Binary — pick a side" is what the row says, so it has to be true. git's + // stage comes back as a lossy UTF-8 decode unless it is read as bytes. + it('answers a binary row from the index, byte for byte', async () => { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-session-bin2-')) + dirs.push(dir) + const git = (...a) => execFileSync('git', a, { cwd: dir, env: { ...process.env, ...GIT_ENV } }) + const theirs = Buffer.from([0x89, 0x50, 0x00, 0xff, 0xfe, 0x0a]) + git('init', '-q', '-b', 'main') + writeFileSync(join(dir, 'logo.png'), Buffer.from([0x89, 0x50, 0x00, 0x01])) + git('add', '.') + git('commit', '-qm', 'base') + git('checkout', '-qb', 'feature') + writeFileSync(join(dir, 'logo.png'), theirs) + git('commit', '-qam', 'theirs') + git('checkout', '-q', 'main') + writeFileSync(join(dir, 'logo.png'), Buffer.from([0x89, 0x50, 0x00, 0x07])) + git('commit', '-qam', 'ours') + try { + git('merge', 'feature') + } catch { + // Expected. + } + await openConflictSession(dir) + expect(await takeSideAt({ index: 0, side: 'theirs' })).toMatchObject({ ok: true }) + expect(readFileSync(join(dir, 'logo.png')).equals(theirs)).toBe(true) + }) +}) + +// git escapes a non-ASCII path in its own output unless told not to. Read back +// as `"\303\274..."` the row shows octal, is misclassified, and the launch's own +// file cannot be found in the list at all. +describe('a path git would otherwise escape', () => { + it('lists a non-ASCII filename as itself', async () => { + const name = 'ünïcødé-文件.txt' + const repo = conflictedRepo([name, 'alpha.txt']) + await openConflictSession(repo.dir) + expect(conflictRows().map((r) => r.name)).toContain(name) + expect(indexOfRel(name)).toBeGreaterThanOrEqual(0) + expect(conflictRows().find((r) => r.name === name).state).toBe('open') + }) +}) diff --git a/tests/main/mergeSession.test.js b/tests/main/mergeSession.test.js index 1f402dc..fed6511 100644 --- a/tests/main/mergeSession.test.js +++ b/tests/main/mergeSession.test.js @@ -8,6 +8,7 @@ import { doneSentinel, endMerge, mergeInProgress, + releaseLauncher, writeMerged } from '../../src/main/mergeSession' @@ -131,3 +132,21 @@ describe('the sentinel a successful write leaves', () => { expect(existsSync(doneSentinel(merged))).toBe(true) }) }) + +describe('releaseLauncher', () => { + // A file answered OUT OF ORDER still gets a launch of its own later, and that + // launch has no session to spend: it is released before any window is shown. + // Without this the terminal running `git mergetool` waits out the launcher's + // two hours on a decision that was already made. + it('releases a launch that has no session behind it', () => { + const dir = scratch() + const merged = join(dir, 'already-done.js') + expect(mergeInProgress()).toBe(null) + expect(releaseLauncher(merged, 'written')).toBe(true) + expect(readFileSync(doneSentinel(merged), 'utf8')).toBe('written') + }) + + it('reports a sentinel it could not write rather than throwing', () => { + expect(releaseLauncher(join(scratch(), 'no', 'such', 'dir', 'f.js'), 'written')).toBe(false) + }) +}) diff --git a/tests/renderer/features/merge/conflictsStore.test.js b/tests/renderer/features/merge/conflictsStore.test.js new file mode 100644 index 0000000..a645880 --- /dev/null +++ b/tests/renderer/features/merge/conflictsStore.test.js @@ -0,0 +1,189 @@ +// The list of conflicted files: what opens it, what closes it, and what happens +// when a row is answered. Rows carry no path — every call names an INDEX, and +// these tests are what hold that boundary. +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' +import { useConflictsStore } from '../../../../src/renderer/src/features/merge/conflictsStore' + +const row = (name, state = 'open', tally = 1) => ({ name, dir: 'src', tally, state }) + +const WALK = { + conflicts: [row('a.js'), row('b.js', 'done', 0), row('icon.png', 'blocked', 0)], + at: 1 +} + +beforeEach(() => { + setActivePinia(createPinia()) + window.api = { + conflictList: vi.fn(async () => WALK.conflicts), + takeConflictSide: vi.fn(async () => ({ ok: true, wasCurrent: false })), + openConflict: vi.fn(async () => ({ ok: true, fileName: 'a.js', content: 'x' })), + endMergeWalk: vi.fn(async () => {}) + } +}) + +describe('begin', () => { + it('takes the rows and the launch’s own position from the payload', () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + expect(conflicts.rows).toHaveLength(3) + expect(conflicts.at).toBe(1) + expect(conflicts.live).toBe(true) + expect(conflicts.open).toBe(true) + }) + + // One file is not a list, and a dialog to choose from it is only in the way. + it('does not open for a single conflicted file', () => { + const conflicts = useConflictsStore() + conflicts.begin({ conflicts: [row('only.js')], at: 0 }) + expect(conflicts.open).toBe(false) + expect(conflicts.isWalk).toBe(false) + }) + + it('survives a payload with no list at all', () => { + const conflicts = useConflictsStore() + conflicts.begin({}) + expect(conflicts.rows).toEqual([]) + expect(conflicts.open).toBe(false) + }) + + it('counts what is left and what is done', () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + expect(conflicts.remaining).toBe(2) + expect(conflicts.done).toBe(1) + }) +}) + +describe('showing it again', () => { + // The bug this feature was asked to fix: dismissing the list used to be the + // end of it. Closing it must leave the walk alive. + it('closing the list leaves the session live, and show brings it back', () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + conflicts.dismiss() + expect(conflicts.open).toBe(false) + expect(conflicts.live).toBe(true) + conflicts.show() + expect(conflicts.open).toBe(true) + }) + + // git launches once per file, so a thirty-file walk used to put the same + // modal in front of the reader thirty times. + it('stays away for the rest of the walk once put away', () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + conflicts.dismiss() + conflicts.begin(WALK) + expect(conflicts.open).toBe(false) + }) + + // Stepping INTO a row is not putting the list away: the next launch, and the + // save that returns here, must still show it. + it('comes back after a row was opened from it', () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + conflicts.hide() + expect(conflicts.open).toBe(false) + conflicts.reveal() + expect(conflicts.open).toBe(true) + }) + + it('offers itself again after a walk ends and another begins', async () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + conflicts.dismiss() + await conflicts.end() + conflicts.begin(WALK) + expect(conflicts.open).toBe(true) + }) + + // git has stopped walking, so there is nothing to reconstruct. + it('refuses to show anything once the walk has ended', async () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + await conflicts.end() + conflicts.show() + expect(conflicts.open).toBe(false) + expect(conflicts.live).toBe(false) + expect(window.api.endMergeWalk).toHaveBeenCalled() + }) +}) + +describe('take', () => { + it('answers a row and re-reads the list from main', async () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + await conflicts.take(0, 'theirs') + expect(window.api.takeConflictSide).toHaveBeenCalledWith(0, 'theirs') + expect(window.api.conflictList).toHaveBeenCalled() + expect(conflicts.error).toBe('') + }) + + // Answering the file the view has open releases that launch in main, so the + // caller has to be told — otherwise the view sits over a merge that is over. + it('reports when the row answered was the one being viewed', async () => { + window.api.takeConflictSide = vi.fn(async () => ({ ok: true, wasCurrent: true })) + const conflicts = useConflictsStore() + conflicts.begin(WALK) + expect(await conflicts.take(1, 'ours')).toBe(true) + }) + + // A row can go stale between the dialog opening and the click. Main refuses + // it; the reader has to see why rather than a row that silently did nothing. + it('surfaces a row git no longer calls conflicted', async () => { + window.api.takeConflictSide = vi.fn(async () => ({ ok: false, error: 'gone' })) + const conflicts = useConflictsStore() + conflicts.begin(WALK) + await conflicts.take(0, 'ours') + expect(conflicts.error).toBe('gone') + }) + + it('refuses to run two at once', async () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + conflicts.busy = true + await conflicts.take(0, 'ours') + expect(window.api.takeConflictSide).not.toHaveBeenCalled() + }) +}) + +describe('payloadAt', () => { + it('hands back what the three-way view needs', async () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + expect(await conflicts.payloadAt(0)).toMatchObject({ fileName: 'a.js' }) + expect(window.api.openConflict).toHaveBeenCalledWith(0) + }) + + it('says so rather than opening a file that cannot be read as text', async () => { + window.api.openConflict = vi.fn(async () => ({ ok: false, error: 'blocked' })) + const conflicts = useConflictsStore() + conflicts.begin(WALK) + expect(await conflicts.payloadAt(2)).toBe(null) + expect(conflicts.error).toBe('blocked') + }) +}) + +describe('select', () => { + it('moves to a row that exists and ignores one that does not', () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + conflicts.select(2) + expect(conflicts.at).toBe(2) + for (const bad of [-1, 3, 99]) { + conflicts.select(bad) + expect(conflicts.at).toBe(2) + } + }) +}) + +describe('markDone', () => { + it('settles a row without waiting for main to be asked again', () => { + const conflicts = useConflictsStore() + conflicts.begin(WALK) + conflicts.markDone(0) + expect(conflicts.rows[0]).toMatchObject({ state: 'done', tally: 0 }) + expect(conflicts.markDone(99)).toBeUndefined() + }) +}) diff --git a/tests/renderer/features/merge/fakeEditor.js b/tests/renderer/features/merge/fakeEditor.js index 23688a1..c91b75b 100644 --- a/tests/renderer/features/merge/fakeEditor.js +++ b/tests/renderer/features/merge/fakeEditor.js @@ -39,7 +39,14 @@ export function fakeEditor(text = '') { onDidScrollChange: on('scroll'), onMouseDown: on('mouse'), onDidChangeModelContent: on('change'), - getScrollTop: () => 120, + // 20px lines and a fixed viewport, so the take buttons' arithmetic has + // something to land on. `top` is writable: a test scrolls by setting it. + getTopForLineNumber: (line) => (line - 1) * 20, + getLayoutInfo: () => ({ height: 400 }), + top: 0, + getScrollTop() { + return this.top + }, setScrollTop: vi.fn(), revealLineInCenter: vi.fn(), setPosition: vi.fn(), diff --git a/tests/renderer/features/merge/mergeDecorations.test.js b/tests/renderer/features/merge/mergeDecorations.test.js index f6623ec..5cd07ce 100644 --- a/tests/renderer/features/merge/mergeDecorations.test.js +++ b/tests/renderer/features/merge/mergeDecorations.test.js @@ -25,15 +25,23 @@ describe('regionOptions', () => { describe('sideDecorations', () => { const regions = [{ ours: ['replicas: 5'], theirs: ['replicas: 9'] }] - it('bands the region and hangs the chevron in the glyph margin', () => { + it('bands the region', () => { const [band] = sideDecorations([{ line: 4, count: 1 }], regions, 'ours', COLORS) expect(band.range.startLineNumber).toBe(4) - expect(band.options.glyphMarginClassName).toContain('merge-take-ours') // Its own colour, not the middle pane's: that one is about state. expect(band.options.className).toBe('merge-side-ours') expect(band.options.overviewRuler.color).toBe(COLORS.ours) }) + // The take control is a real button on the pane's INNER edge + // (MergeTakeOverlay), because a glyph margin only exists on an editor's left + // edge and cannot be reached from the keyboard. Nothing may put one back. + it('hangs nothing in the glyph margin', () => { + const [band] = sideDecorations([{ line: 4, count: 1 }], regions, 'ours', COLORS) + expect(band.options.glyphMarginClassName).toBeUndefined() + expect(band.options.glyphMarginHoverMessage).toBeUndefined() + }) + it('tints only the columns that differ, at the side pane’s own line numbers', () => { const decorations = sideDecorations([{ line: 4, count: 1 }], regions, 'theirs', COLORS) const word = decorations.find((d) => d.options.className?.includes('merge-word')) diff --git a/tests/renderer/features/merge/mergePaneOps.test.js b/tests/renderer/features/merge/mergePaneOps.test.js index 8fbe4d3..a18a079 100644 --- a/tests/renderer/features/merge/mergePaneOps.test.js +++ b/tests/renderer/features/merge/mergePaneOps.test.js @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it } from 'vitest' import { fakeEditor } from './fakeEditor' import { remember, @@ -7,9 +7,10 @@ import { reveal, seedRegions, syncScroll, - takeFromGutter, + takeLayout, writeChoice } from '../../../../src/renderer/src/features/merge/mergePaneOps' +import * as ops from '../../../../src/renderer/src/features/merge/mergePaneOps' function scene() { const merge = { @@ -105,23 +106,33 @@ describe('resolveTouched', () => { }) }) -describe('takeFromGutter', () => { - const anchors = { ours: [{ line: 2, count: 1 }] } - const glyph = (lineNumber) => ({ - target: { type: 2, position: { lineNumber } } +describe('takeLayout', () => { + const anchors = { ours: [{ line: 2, count: 1 }], theirs: [{ line: 2, count: 1 }] } + const merge = { at: 0 } + + // Geometry comes off the live editor, which is why only the mapping is + // testable here; where the buttons actually land is proven in e2e. + const editorAt = (top) => ({ + getTopForLineNumber: (line) => (line - 1) * 20, + getScrollTop: () => top, + getLayoutInfo: () => ({ height: 400 }) + }) + + it('places one button per region the side contains', () => { + const editors = { ours: editorAt(0) } + expect(takeLayout({ editors, anchors, merge, key: 'ours' })).toEqual([ + { index: 0, top: 20, dim: false } + ]) }) - it('answers the region the click sits on', () => { - const take = vi.fn() - takeFromGutter({ anchors, key: 'ours', event: glyph(2), take }) - expect(take).toHaveBeenCalledWith(0, 'ours') + it('answers nothing before the editors exist', () => { + expect(takeLayout({ editors: {}, anchors, merge, key: 'ours' })).toEqual([]) }) - it('ignores a click on a line no region covers, and one outside the margin', () => { - const take = vi.fn() - takeFromGutter({ anchors, key: 'ours', event: glyph(9), take }) - takeFromGutter({ anchors, key: 'ours', event: { target: { type: 6 } }, take }) - expect(take).not.toHaveBeenCalled() + // The glyph margin this replaced could not be reached from the keyboard, and + // only existed on an editor's left edge. Nothing may go back to it. + it('no longer reads a glyph-margin click', () => { + expect(ops.takeFromGutter).toBeUndefined() }) }) @@ -129,6 +140,7 @@ describe('syncScroll', () => { it('moves the other two panes and does not echo back', () => { const s = scene() const sync = { busy: false } + s.editors.result.top = 120 syncScroll(s.editors, 'result', sync) expect(s.editors.ours.setScrollTop).toHaveBeenCalledWith(120) expect(s.editors.theirs.setScrollTop).toHaveBeenCalledWith(120) diff --git a/tests/renderer/features/merge/mergeTakeOverlay.test.js b/tests/renderer/features/merge/mergeTakeOverlay.test.js new file mode 100644 index 0000000..2adcfc5 --- /dev/null +++ b/tests/renderer/features/merge/mergeTakeOverlay.test.js @@ -0,0 +1,71 @@ +// The take buttons' geometry. Real Monaco moves these on every scroll and every +// edit; what is testable without one is which regions get a button, where it +// lands, and what gets clipped. +import { describe, expect, it } from 'vitest' +import { takeButtons } from '../../../../src/renderer/src/features/merge/mergeTakeOverlay' + +// 20px lines, the way the editor lays them out. +const topOf = (line) => (line - 1) * 20 + +const view = (over) => ({ + anchors: [ + { line: 2, count: 2 }, + { line: 10, count: 1 } + ], + topOf, + scrollTop: 0, + height: 400, + current: 0, + ...over +}) + +describe('takeButtons', () => { + it('puts one button at the top of each band', () => { + expect(takeButtons(view())).toEqual([ + { index: 0, top: 20, dim: false }, + { index: 1, top: 180, dim: true } + ]) + }) + + it('keeps the index of the region, not of the button', () => { + const anchors = [null, { line: 4, count: 1 }] + expect(takeButtons(view({ anchors }))).toEqual([{ index: 1, top: 60, dim: true }]) + }) + + // The side deleted those lines, so there is no band and nothing to take. + it('draws nothing for a region the side does not contain', () => { + expect(takeButtons(view({ anchors: [null, null] }))).toEqual([]) + }) + + it('follows the scroll', () => { + expect(takeButtons(view({ scrollTop: 10 }))).toEqual([ + { index: 0, top: 10, dim: false }, + { index: 1, top: 170, dim: true } + ]) + }) + + // Left in the DOM it would still be focusable, which is how an overlay button + // ends up floating over the pane header. + it('drops a button scrolled out of the pane', () => { + expect(takeButtons(view({ scrollTop: 185 })).map((b) => b.index)).toEqual([1]) + }) + + it('drops a button below the fold', () => { + expect(takeButtons(view({ height: 100 })).map((b) => b.index)).toEqual([0]) + }) + + // Half a button over the top edge is still worth showing: the band it belongs + // to is on screen even when its first line is not quite. + it('keeps a button straddling the top edge', () => { + expect(takeButtons(view({ scrollTop: 28 }))[0]).toEqual({ index: 0, top: -8, dim: false }) + expect(takeButtons(view({ scrollTop: 32 })).map((b) => b.index)).toEqual([1]) + }) + + it('marks every region but the current one as receding', () => { + expect(takeButtons(view({ current: 1 })).map((b) => b.dim)).toEqual([true, false]) + }) + + it('answers nothing when the side has no anchors yet', () => { + expect(takeButtons(view({ anchors: undefined }))).toEqual([]) + }) +}) diff --git a/tests/renderer/features/merge/useMergePanes.test.js b/tests/renderer/features/merge/useMergePanes.test.js index 95cb562..2144bd4 100644 --- a/tests/renderer/features/merge/useMergePanes.test.js +++ b/tests/renderer/features/merge/useMergePanes.test.js @@ -68,14 +68,28 @@ describe('useMergePanes', () => { expect(merge.remaining).toBe(0) }) - it('follows a chevron clicked in a side pane', () => { - const { merge, ours } = scene() - ours.fire('mouse', { target: { type: 2, position: { lineNumber: 2 } } }) + // The take control is a button the view renders over the pane, so what the + // composable owes it is a position per region — and a fresh one after a + // scroll, since the buttons move with the text. + it('offers a take button for each side, and re-places them on scroll', () => { + const { panes, ours } = scene() + expect(panes.takes.value.ours).toEqual([{ index: 0, top: 20, dim: false }]) + expect(panes.takes.value.theirs).toEqual([{ index: 0, top: 20, dim: false }]) + ours.top = 20 + ours.fire('scroll') + expect(panes.takes.value.ours).toEqual([{ index: 0, top: 0, dim: false }]) + }) + + it('takes that side when the view reports its button pressed', () => { + const { panes, merge, result } = scene() + panes.take(panes.takes.value.theirs[0].index, 'theirs') + expect(result.__lines().join('\n')).toContain('replicas: 9') expect(merge.regions[0].resolved).toBe(true) }) it('carries a scroll in one pane to the other two', () => { const { ours, result, theirs } = scene() + ours.top = 120 ours.fire('scroll') expect(result.setScrollTop).toHaveBeenCalledWith(120) expect(theirs.setScrollTop).toHaveBeenCalledWith(120) diff --git a/tests/renderer/utils/detectLanguage.test.js b/tests/renderer/utils/detectLanguage.test.js index af0cbef..b4b573c 100644 --- a/tests/renderer/utils/detectLanguage.test.js +++ b/tests/renderer/utils/detectLanguage.test.js @@ -147,9 +147,9 @@ describe('detectSnippetLanguage — programming languages', () => { ) ).toBe('php') expect(detect('$appId = "ABCD";\n$apiToken = "EFGH";')).toBe('php') - expect(detect("$payload = [\n 'user_ids' => $userIds,\n 'is_distinct' => true,\n];")).toBe( - 'php' - ) + expect( + detect("$payload = [\n 'user_ids' => $userIds,\n 'is_distinct' => true,\n];") + ).toBe('php') expect(detect("require_once __DIR__ . '/vendor/autoload.php';")).toBe('php') }) @@ -158,15 +158,17 @@ describe('detectSnippetLanguage — programming languages', () => { // spaces around it is prose. Every case here detected as PHP once. it('does not claim other languages for PHP', () => { expect(detect('use std::io;\n\nfn main() {\n println!("hi");\n}')).toBe('rust') - expect(detect('const $el = document.querySelector("#app")\nconsole.log($el)')).toBe('javascript') + expect(detect('const $el = document.querySelector("#app")\nconsole.log($el)')).toBe( + 'javascript' + ) expect(detect('name=diffbro\necho "building $name"\nexport OUT=$name.zip')).toBe('shell') // PHP writes the type BEFORE the sigil (`private string $x;`); a `$name` // followed by a colon is a TypeScript annotation, not a PHP property. expect(detect('class Widget {\n private $el: JQuery\n show() {}\n}')).toBe('typescript') // A jQuery-flavoured parameter name does not make a function PHP. - expect( - detect("function bindTabs($root) {\n $root.find('.tab').on('click', toggle)\n}") - ).toBe('javascript') + expect(detect("function bindTabs($root) {\n $root.find('.tab').on('click', toggle)\n}")).toBe( + 'javascript' + ) expect(detect('export function render($el: HTMLElement): void {\n $el.remove()\n}')).toBe( 'javascript' ) @@ -188,9 +190,9 @@ describe('detectSnippetLanguage — programming languages', () => { // case here was mislabelled php — which then stuck, because an auto snippet // records its verdict on save. it('does not claim prose and release notes that mention PHP', () => { - expect( - detect('# 2.1.0\n\n- Dropped the `require_once` bootstrap.\n- Bumped minimums.') - ).toBe('markdown') + expect(detect('# 2.1.0\n\n- Dropped the `require_once` bootstrap.\n- Bumped minimums.')).toBe( + 'markdown' + ) expect(detect('fix: drop the require_once shim\n\nCloses #12')).toBe('plaintext') expect(detect("# Config\n\nSet it:\n\n $config['url'] = 'https://x';\n\nDone.")).toBe( 'markdown' diff --git a/tests/renderer/utils/monacoTheme.test.js b/tests/renderer/utils/monacoTheme.test.js index 81b5597..a294090 100644 --- a/tests/renderer/utils/monacoTheme.test.js +++ b/tests/renderer/utils/monacoTheme.test.js @@ -37,9 +37,7 @@ describe('monacoThemeData', () => { it('gives two themes two different diffs', () => { const a = monacoThemeData(tide, { dark: true }).colors const b = monacoThemeData(amber, { dark: true }).colors - expect(a['diffEditor.insertedLineBackground']).not.toBe( - b['diffEditor.insertedLineBackground'] - ) + expect(a['diffEditor.insertedLineBackground']).not.toBe(b['diffEditor.insertedLineBackground']) expect(a['editor.background']).not.toBe(b['editor.background']) }) diff --git a/tests/renderer/utils/shortcuts.test.js b/tests/renderer/utils/shortcuts.test.js index 92404c3..e49a7de 100644 --- a/tests/renderer/utils/shortcuts.test.js +++ b/tests/renderer/utils/shortcuts.test.js @@ -109,3 +109,24 @@ describe('the shortcuts dialog against the menus that bind the keys', () => { expect(stale, 'advertised in the dialog but bound by nothing').toEqual([]) }) }) + +// The collision that shipped: Cmd+Shift+M was bound by Tools ▸ XML and then +// bound AGAIN by a new View item. Electron gives the first registration the +// key, so the second menu item silently stopped working — and the walker above +// de-dupes into a Map, so nothing failed. This is what would have caught it. +describe('no two menu items claim the same key', () => { + it('binds each accelerator exactly once', () => { + const claims = new Map() + const walk = (items) => { + for (const item of items ?? []) { + if (item.keys) claims.set(item.keys, [...(claims.get(item.keys) ?? []), item.label]) + walk(item.items ?? item.children) + } + } + walk(buildMenus(() => {})) + const doubled = [...claims] + .filter(([, labels]) => labels.length > 1) + .map(([keys, labels]) => `${keys} → ${labels.join(' AND ')}`) + expect(doubled, 'two menu items bound to one key').toEqual([]) + }) +}) diff --git a/tests/renderer/utils/snippetState.test.js b/tests/renderer/utils/snippetState.test.js index 85bc92f..d165274 100644 --- a/tests/renderer/utils/snippetState.test.js +++ b/tests/renderer/utils/snippetState.test.js @@ -45,9 +45,9 @@ describe('derivedFrom — prompt variables', () => { it('keeps a list it already has rather than recomputing it', () => { const vars = ['kept'] - expect(derivedFrom({ language: 'claude', detected: 'claude', vars }, 'Review {{file}}').vars).toBe( - vars - ) + expect( + derivedFrom({ language: 'claude', detected: 'claude', vars }, 'Review {{file}}').vars + ).toBe(vars) }) // The two fields answer the same question, so a re-detection that moves one diff --git a/tests/stubs/monaco-editor.js b/tests/stubs/monaco-editor.js index 47737f6..3f883e4 100644 --- a/tests/stubs/monaco-editor.js +++ b/tests/stubs/monaco-editor.js @@ -9,7 +9,6 @@ export class Range { export const editor = { OverviewRulerLane: { Left: 1, Center: 2, Right: 4, Full: 7 }, - MouseTargetType: { GUTTER_GLYPH_MARGIN: 2 }, create: () => ({ dispose: () => {} }), createDiffEditor: () => ({ dispose: () => {} }), colorizeElement: async () => {}, diff --git a/vitest.config.mjs b/vitest.config.mjs index a4137a9..f8793fe 100644 --- a/vitest.config.mjs +++ b/vitest.config.mjs @@ -81,6 +81,8 @@ export default defineConfig({ 'src/main/clipboardStage.js', 'src/main/clipboardFiles.js', 'src/main/demoContent.js', + 'src/main/conflictList.js', + 'src/main/conflictSession.js', 'src/main/xlsx/**', 'src/shared/**', 'src/renderer/src/stores/**',