From e81854da9abe2d44ed61bafbbcef5c032c17f7c5 Mon Sep 17 00:00:00 2001 From: Amir Date: Sat, 16 May 2026 18:19:53 +0200 Subject: [PATCH] feat!: remove Jujutsu support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dunk now targets Git only. Keeping the VCS layer to a single backend removes the jj loaders, the `vcs` config key and auto-detection, the jj branch-review revset path, and jj watch signatures — less surface to maintain for a personal tool. jj users can run a colocated Git checkout. BREAKING CHANGE: the `vcs` config option is gone and jj workspaces are no longer auto-detected. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 4 + README.md | 23 +--- src/core/branchReview.test.ts | 28 +---- src/core/branchReview.ts | 26 ----- src/core/cli.ts | 2 +- src/core/cliComments.test.ts | 2 +- src/core/cliComments.ts | 2 +- src/core/config.test.ts | 84 -------------- src/core/config.ts | 28 +---- src/core/jj.test.ts | 171 ----------------------------- src/core/jj.ts | 201 ---------------------------------- src/core/loaders.test.ts | 112 ------------------- src/core/loaders.ts | 70 +----------- src/core/types.ts | 2 - src/core/watch.ts | 25 +---- 15 files changed, 15 insertions(+), 765 deletions(-) delete mode 100644 src/core/jj.test.ts delete mode 100644 src/core/jj.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 287f91e..f9ec73b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ## [Unreleased] +### Removed + +- Jujutsu support. `dunk` now targets Git only — there's no more `vcs` config key, jj workspace auto-detection, or jj revset handling for `dunk diff`/`dunk show`. Use a colocated Git checkout if you work in jj. + ## [0.13.0] - 2026-05-14 ### Added diff --git a/README.md b/README.md index 187177d..84ab9e2 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ git config --global alias.dshow "-c core.pager=\"dunk pager\" show" ## Whole-branch review -`dunk diff --branch` shows everything that differs between the current branch and its base — committed history, staged work, unstaged edits, and untracked files — in one review. Pick the base explicitly with `--branch=`, or let `dunk` resolve it: explicit flag → `[branch_review] base` in `.dunk/config.toml` → `origin/HEAD` → `origin/main` / `main` / `origin/master` / `master` / `origin/trunk` / `trunk`. The resolved base is shown in the status bar so auto-detection is never silent. Works in Jujutsu too (uses a `fork_point(@ | "")` revset). +`dunk diff --branch` shows everything that differs between the current branch and its base — committed history, staged work, unstaged edits, and untracked files — in one review. Pick the base explicitly with `--branch=`, or let `dunk` resolve it: explicit flag → `[branch_review] base` in `.dunk/config.toml` → `origin/HEAD` → `origin/main` / `main` / `origin/master` / `master` / `origin/trunk` / `trunk`. The resolved base is shown in the status bar so auto-detection is never silent. ```toml # .dunk/config.toml @@ -153,26 +153,6 @@ git config --global alias.dshow "-c core.pager=\"dunk pager\" show" base = "origin/main" ``` -## Jujutsu - -`dunk` auto-detects Jujutsu workspaces. Inside one, `dunk diff [revset]` and `dunk show [revset]` use jj revsets. - -To force a backend, set `vcs = "git"` or `vcs = "jj"` in [config](#config). - -To use `dunk` as jj’s pager, run: - -```bash -jj config edit --user -``` - -Then add: - -```toml -[ui] -pager = ["dunk", "pager"] -diff-formatter = ":git" -``` - ## Config `dunk` reads config from either location: @@ -185,7 +165,6 @@ Example: ```toml theme = "graphite" # graphite, midnight, paper, ember mode = "auto" # auto, split, stack -vcs = "git" # git, jj exclude_untracked = false line_numbers = false wrap_lines = true diff --git a/src/core/branchReview.test.ts b/src/core/branchReview.test.ts index bdfe1cb..efead12 100644 --- a/src/core/branchReview.test.ts +++ b/src/core/branchReview.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { resolveGitBranchBase, resolveJjBranchBase } from "./branchReview"; +import { resolveGitBranchBase } from "./branchReview"; import type { VcsCommandInput } from "./types"; const tempDirs: string[] = []; @@ -149,29 +149,3 @@ describe("resolveGitBranchBase", () => { ).toThrow(/common ancestor/); }); }); - -describe("resolveJjBranchBase", () => { - test("wraps an explicit base in a fork_point revset", () => { - const resolved = resolveJjBranchBase( - buildVcsInput({ branchReview: { explicitBase: "origin/main" } }), - ); - - expect(resolved.displayBase).toBe("origin/main"); - expect(resolved.jjFromRevset).toBe('fork_point(@ | "origin/main")'); - }); - - test("falls back to trunk() when nothing else is configured", () => { - const resolved = resolveJjBranchBase(buildVcsInput({ branchReview: {} })); - - expect(resolved.displayBase).toBe("trunk()"); - expect(resolved.jjFromRevset).toBe('fork_point(@ | "trunk()")'); - }); - - test("escapes embedded quotes so the revset stays parseable", () => { - const resolved = resolveJjBranchBase( - buildVcsInput({ branchReview: { explicitBase: 'weird"name' } }), - ); - - expect(resolved.jjFromRevset).toBe('fork_point(@ | "weird\\"name")'); - }); -}); diff --git a/src/core/branchReview.ts b/src/core/branchReview.ts index 7f97679..24f1bde 100644 --- a/src/core/branchReview.ts +++ b/src/core/branchReview.ts @@ -22,13 +22,10 @@ const GIT_BASE_FALLBACK_REFS = [ * - `displayBase` is what we show to the user (e.g. "origin/main"). * - `gitMergeBaseSha` is the SHA of `git merge-base HEAD`, used as the diff target * so the diff is "working tree vs the common ancestor" rather than "working tree vs the tip". - * - `jjFromRevset` is the `--from` argument for `jj diff` — `fork_point()`, which - * gives the same merge-base semantics that GitHub's three-dot view uses. */ export interface ResolvedBranchBase { displayBase: string; gitMergeBaseSha?: string; - jjFromRevset?: string; } /** Trim and discard one-line output from `git rev-parse` / `git merge-base`. */ @@ -255,26 +252,3 @@ export function resolveGitBranchBase( return resolveExplicitGitBase(detected, cwd, gitExecutable); } - -/** Build the Jujutsu `fork_point` revset used as the `--from` argument for branch review. */ -function jjForkPointRevset(base: string) { - // Jujutsu's revset language uses double quotes for literal strings, so wrap whatever the user - // (or config) named so revsets with slashes or hyphens — e.g. "origin/main" — parse cleanly. - const escaped = base.replace(/\\/g, "\\\\").replace(/"/g, '\\"'); - return `fork_point(@ | "${escaped}")`; -} - -/** Resolve a Jujutsu branch-review base. */ -export function resolveJjBranchBase(input: VcsCommandInput): ResolvedBranchBase { - const explicitBase = input.branchReview?.explicitBase; - const configuredBase = input.options.branchReviewBase; - const base = explicitBase ?? configuredBase ?? "trunk()"; - - // For Jujutsu we keep resolution lazy: the actual revset evaluation happens inside `jj diff`, - // which already raises a clear "Revision not found" / "Failed to parse revset" error when the - // base is unresolvable. Catching that here would duplicate jj.ts error translation. - return { - displayBase: base, - jjFromRevset: jjForkPointRevset(base), - }; -} diff --git a/src/core/cli.ts b/src/core/cli.ts index 7258321..0b031ad 100644 --- a/src/core/cli.ts +++ b/src/core/cli.ts @@ -147,7 +147,7 @@ function renderCliHelp() { "", "Notes:", " Run `dunk --help` for command-specific syntax and options.", - ' "target" refers to a generic set of changes; it can be a ref (git) or revset (jj)', + ' "target" refers to a generic set of changes; it can be any git ref or commit range', "", ].join("\n"); } diff --git a/src/core/cliComments.test.ts b/src/core/cliComments.test.ts index 717180d..35058b5 100644 --- a/src/core/cliComments.test.ts +++ b/src/core/cliComments.test.ts @@ -354,7 +354,7 @@ describe("dunk comments CLI", () => { test("commands fail clearly when the cwd is not a repo", () => { const dir = mkdtempSync(join(tmpdir(), "dunk-cli-no-repo-")); tempDirs.push(dir); - expect(() => runCommentsList("text", { cwd: dir })).toThrow(/git or jj repository/); + expect(() => runCommentsList("text", { cwd: dir })).toThrow(/Not inside a git repository/); }); test("renderCommentsHelp lists every subcommand", () => { diff --git a/src/core/cliComments.ts b/src/core/cliComments.ts index 4eb79de..5e6152d 100644 --- a/src/core/cliComments.ts +++ b/src/core/cliComments.ts @@ -32,7 +32,7 @@ interface CommentsShowOptions extends CommentsListOptions { function requireRepoRoot(cwd?: string): string { const repoRoot = findRepoRoot(cwd ?? process.cwd()); if (!repoRoot) { - throw new DunkUserError("Not inside a git or jj repository.", [ + throw new DunkUserError("Not inside a git repository.", [ "`dunk comments` reads `.dunk/comments.json` from the repo root; run it from inside a checkout.", ]); } diff --git a/src/core/config.test.ts b/src/core/config.test.ts index 2ab3540..ce7ac1e 100644 --- a/src/core/config.test.ts +++ b/src/core/config.test.ts @@ -27,10 +27,6 @@ function createRepo(dir: string) { mkdirSync(join(dir, ".git"), { recursive: true }); } -function createJjRepo(dir: string) { - mkdirSync(join(dir, ".jj"), { recursive: true }); -} - function createPatchPagerInput(overrides: Partial = {}): CliInput { return { kind: "patch", @@ -160,86 +156,6 @@ describe("config resolution", () => { expect(fallbackResolved.input.options.excludeUntracked).toBe(false); }); - test("defaults to git VCS mode and accepts jj from config", () => { - const home = createTempDir("hunk-config-home-"); - mkdirSync(join(home, ".config", "dunk"), { recursive: true }); - writeFileSync(join(home, ".config", "dunk", "config.toml"), 'vcs = "jj"\n'); - - const cwd = createTempDir("hunk-config-cwd-"); - const defaultResolved = resolveConfiguredCliInput( - { - kind: "vcs", - staged: false, - options: {}, - }, - { cwd, env: { HOME: createTempDir("hunk-config-empty-home-") } }, - ); - const configuredResolved = resolveConfiguredCliInput( - { - kind: "vcs", - staged: false, - options: {}, - }, - { cwd, env: { HOME: home } }, - ); - - expect(defaultResolved.input.options.vcs).toBe("git"); - expect(configuredResolved.input.options.vcs).toBe("jj"); - }); - - test("auto-detects jj checkouts before falling back to git mode", () => { - const home = createTempDir("hunk-config-home-"); - const jjRepo = createTempDir("hunk-config-jj-repo-"); - const colocatedRepo = createTempDir("hunk-config-colocated-repo-"); - const gitRepo = createTempDir("hunk-config-git-repo-"); - const plainDir = createTempDir("hunk-config-no-repo-"); - - createJjRepo(jjRepo); - createRepo(colocatedRepo); - createJjRepo(colocatedRepo); - createRepo(gitRepo); - - const input = { - kind: "vcs", - staged: false, - options: {}, - } satisfies CliInput; - - expect( - resolveConfiguredCliInput(input, { cwd: jjRepo, env: { HOME: home } }).input.options.vcs, - ).toBe("jj"); - expect( - resolveConfiguredCliInput(input, { cwd: colocatedRepo, env: { HOME: home } }).input.options - .vcs, - ).toBe("jj"); - expect( - resolveConfiguredCliInput(input, { cwd: gitRepo, env: { HOME: home } }).input.options.vcs, - ).toBe("git"); - expect( - resolveConfiguredCliInput(input, { cwd: plainDir, env: { HOME: home } }).input.options.vcs, - ).toBe("git"); - }); - - test("explicit config overrides auto-detected jj mode", () => { - const home = createTempDir("hunk-config-home-"); - const repo = createTempDir("hunk-config-jj-repo-"); - createJjRepo(repo); - - mkdirSync(join(repo, ".dunk"), { recursive: true }); - writeFileSync(join(repo, ".dunk", "config.toml"), 'vcs = "git"\n'); - - const resolved = resolveConfiguredCliInput( - { - kind: "vcs", - staged: false, - options: {}, - }, - { cwd: repo, env: { HOME: home } }, - ); - - expect(resolved.input.options.vcs).toBe("git"); - }); - test("loadAppBootstrap exposes resolved initial preferences to the UI", async () => { const home = createTempDir("hunk-config-home-"); const repo = createTempDir("hunk-config-repo-"); diff --git a/src/core/config.ts b/src/core/config.ts index 20f1289..2542f57 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -2,13 +2,7 @@ import fs from "node:fs"; import { dirname, join, resolve } from "node:path"; import { DUNK_CONFIG_RELATIVE_PATH } from "./dunkPaths"; import { resolveGlobalConfigPath } from "./paths"; -import type { - CliInput, - CommonOptions, - LayoutMode, - PersistedViewPreferences, - VcsMode, -} from "./types"; +import type { CliInput, CommonOptions, LayoutMode, PersistedViewPreferences } from "./types"; export const DEFAULT_VIEW_PREFERENCES: PersistedViewPreferences = { mode: "auto", @@ -38,11 +32,6 @@ function normalizeLayoutMode(value: unknown): LayoutMode | undefined { return value === "auto" || value === "split" || value === "stack" ? value : undefined; } -/** Accept only the VCS backends dunk can load directly. */ -function normalizeVcsMode(value: unknown): VcsMode | undefined { - return value === "git" || value === "jj" ? value : undefined; -} - /** Accept only plain booleans from config files. */ function normalizeBoolean(value: unknown) { return typeof value === "boolean" ? value : undefined; @@ -62,7 +51,6 @@ function readConfigPreferences(source: Record): CommonOptions { return { mode: normalizeLayoutMode(source.mode), - vcs: normalizeVcsMode(source.vcs), theme: normalizeString(source.theme), excludeUntracked: normalizeBoolean(source.exclude_untracked), lineNumbers: normalizeBoolean(source.line_numbers), @@ -78,7 +66,6 @@ function mergeOptions(base: CommonOptions, overrides: CommonOptions): CommonOpti return { ...base, mode: overrides.mode ?? base.mode, - vcs: overrides.vcs ?? base.vcs, theme: overrides.theme ?? base.theme, pager: overrides.pager ?? base.pager, watch: overrides.watch ?? base.watch, @@ -113,7 +100,7 @@ export function findRepoRoot(cwd = process.cwd()) { let current = resolve(cwd); for (;;) { - if (fs.existsSync(join(current, ".git")) || fs.existsSync(join(current, ".jj"))) { + if (fs.existsSync(join(current, ".git"))) { return current; } @@ -126,15 +113,6 @@ export function findRepoRoot(cwd = process.cwd()) { } } -/** Choose the VCS backend that best matches the discovered checkout. */ -function detectRepoVcsMode(repoRoot?: string): VcsMode { - if (repoRoot && fs.existsSync(join(repoRoot, ".jj"))) { - return "jj"; - } - - return "git"; -} - /** Parse one TOML config file into a plain object. */ function readTomlRecord(path: string) { if (!fs.existsSync(path)) { @@ -160,7 +138,6 @@ export function resolveConfiguredCliInput( let resolvedOptions: CommonOptions = { mode: DEFAULT_VIEW_PREFERENCES.mode, - vcs: detectRepoVcsMode(repoRoot), // Keep the built-in theme default explicit so stdin-backed startup paths do not depend on // renderer theme-mode detection for their initial palette. theme: "graphite", @@ -194,7 +171,6 @@ export function resolveConfiguredCliInput( pager: input.options.pager ?? false, watch: input.options.watch ?? false, excludeUntracked: resolvedOptions.excludeUntracked ?? false, - vcs: resolvedOptions.vcs ?? "git", }; return { diff --git a/src/core/jj.test.ts b/src/core/jj.test.ts deleted file mode 100644 index ad6f412..0000000 --- a/src/core/jj.test.ts +++ /dev/null @@ -1,171 +0,0 @@ -import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test"; -import { mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { buildJjDiffArgs, runJjText } from "./jj"; - -// jj sub-process spawns are dramatically slower on Windows (~70× the Linux time observed in CI), -// so give the suite generous headroom to absorb that overhead without flaking. -setDefaultTimeout(30_000); - -const tempDirs: string[] = []; -const realJjTest = Bun.which("jj") ? test : test.skip; - -function cleanupTempDirs() { - while (tempDirs.length > 0) { - const dir = tempDirs.pop(); - if (dir) { - rmSync(dir, { recursive: true, force: true }); - } - } -} - -function createTempDir(prefix: string) { - const dir = realpathSync(mkdtempSync(join(tmpdir(), prefix))); - tempDirs.push(dir); - return dir; -} - -function jj(cwd: string, ...cmd: string[]) { - const proc = Bun.spawnSync( - [ - "jj", - "--config", - "signing.behavior=drop", - "--config", - 'user.name="Test User"', - "--config", - "user.email=test@example.com", - ...cmd, - ], - { - cwd, - stdout: "pipe", - stderr: "pipe", - stdin: "ignore", - }, - ); - - if (proc.exitCode !== 0) { - const stderr = Buffer.from(proc.stderr).toString("utf8"); - throw new Error(stderr.trim() || `jj ${cmd.join(" ")} failed`); - } - - return Buffer.from(proc.stdout).toString("utf8"); -} - -function createTempJjRepo(prefix: string) { - const dir = createTempDir(prefix); - - jj(tmpdir(), "git", "init", "--colocate", dir); - - return dir; -} - -function findDuplicatePrefix(values: string[]) { - const seen = new Set(); - - for (const value of values) { - const prefix = value[0]; - if (!prefix) { - continue; - } - - if (seen.has(prefix)) { - return prefix; - } - - seen.add(prefix); - } - - return undefined; -} - -afterEach(() => { - cleanupTempDirs(); -}); - -describe("jj command helpers", () => { - test("reports a friendly error when jj is not installed or not on PATH", () => { - expect(() => - runJjText({ - input: { - kind: "vcs", - staged: false, - options: { mode: "auto", vcs: "jj" }, - }, - args: ["root"], - jjExecutable: "definitely-not-a-real-jj-binary", - }), - ).toThrow( - 'Jujutsu is required for `dunk diff` when `vcs = "jj"`, but `definitely-not-a-real-jj-binary` was not found in PATH.', - ); - }); - - realJjTest("reports a friendly error outside a jj repository", () => { - const dir = createTempDir("hunk-jj-nonrepo-"); - - expect(() => - runJjText({ - input: { - kind: "vcs", - staged: false, - options: { mode: "auto", vcs: "jj" }, - }, - args: ["root"], - cwd: dir, - }), - ).toThrow('`dunk diff` must be run inside a Jujutsu repository when `vcs = "jj"`.'); - }); - - realJjTest("reports a friendly error for invalid revsets", () => { - const dir = createTempJjRepo("hunk-jj-invalid-revset-"); - const input = { - kind: "vcs" as const, - range: "missing_revision", - staged: false, - options: { mode: "auto" as const, vcs: "jj" as const }, - }; - - expect(() => - runJjText({ - input, - args: buildJjDiffArgs(input), - cwd: dir, - }), - ).toThrow("`dunk diff missing_revision` could not resolve Jujutsu revset `missing_revision`."); - }); - - realJjTest("reports a friendly error for ambiguous change id prefixes", () => { - const dir = createTempJjRepo("hunk-jj-ambiguous-prefix-"); - let prefix: string | undefined; - - for (let index = 0; index < 32 && !prefix; index += 1) { - writeFileSync(join(dir, `file-${index}.txt`), `${index}\n`); - jj(dir, "commit", "-m", `commit ${index}`); - - prefix = findDuplicatePrefix( - jj(dir, "log", "--no-graph", "-T", 'change_id ++ "\n"').trim().split("\n"), - ); - } - - if (!prefix) { - throw new Error("Expected generated jj changes to include an ambiguous prefix."); - } - - const input = { - kind: "vcs" as const, - range: prefix, - staged: false, - options: { mode: "auto" as const, vcs: "jj" as const }, - }; - - expect(() => - runJjText({ - input, - args: buildJjDiffArgs(input), - cwd: dir, - }), - ).toThrow(`\`dunk diff ${prefix}\` could not resolve Jujutsu revset \`${prefix}\`.`); - }); -}); diff --git a/src/core/jj.ts b/src/core/jj.ts deleted file mode 100644 index 9a30ae0..0000000 --- a/src/core/jj.ts +++ /dev/null @@ -1,201 +0,0 @@ -import { DunkUserError } from "./errors"; -import type { VcsCommandInput, ShowCommandInput } from "./types"; - -export type JjBackedInput = VcsCommandInput | ShowCommandInput; - -export interface RunJjTextOptions { - input: JjBackedInput; - args: string[]; - cwd?: string; - jjExecutable?: string; -} - -/** Append Jujutsu filesets only when the caller requested path filtering. */ -function appendJjFilesets(args: string[], pathspecs?: string[]) { - if (!pathspecs || pathspecs.length === 0) { - return; - } - - args.push("--", ...pathspecs); -} - -/** Build the `jj diff --git` arguments for working-copy and revset reviews. */ -export function buildJjDiffArgs(input: VcsCommandInput, jjFromRevset?: string) { - const args = ["diff", "--git"]; - - if (jjFromRevset) { - // Branch review path: `jj diff --from fork_point(@ | "base") --to @` mirrors GitHub's - // three-dot semantics. We deliberately drop `-r` here so `--from`/`--to` are not mixed - // with single-revset selection, which jj rejects. - args.push("--from", jjFromRevset, "--to", "@"); - } else if (input.range) { - args.push("-r", input.range); - } - - appendJjFilesets(args, input.pathspecs); - return args; -} - -/** Build the `jj diff --git -r` arguments used for `dunk show` in Jujutsu mode. */ -export function buildJjShowArgs(input: ShowCommandInput) { - const args = ["diff", "--git", "-r", input.ref ?? "@"]; - - appendJjFilesets(args, input.pathspecs); - return args; -} - -export function formatJjCommandLabel(input: JjBackedInput) { - if (input.kind === "vcs") { - if (input.staged) { - return "dunk diff --staged"; - } - - if (input.branchReview) { - return input.branchReview.explicitBase - ? `dunk diff --branch=${input.branchReview.explicitBase}` - : "dunk diff --branch"; - } - - return input.range ? `dunk diff ${input.range}` : "dunk diff"; - } - - return input.ref ? `dunk show ${input.ref}` : "dunk show"; -} - -function trimJjPrefix(message: string) { - return message.replace(/^error:\s*/i, "").trim(); -} - -function firstJjErrorLine(stderr: string) { - const line = stderr - .split("\n") - .map((entry) => entry.trim()) - .find(Boolean); - - return trimJjPrefix((line ?? stderr.trim()) || "Jujutsu command failed."); -} - -function isMissingJjRepoMessage(stderr: string) { - return ["There is no jj repo in", "not in a workspace"].some((fragment) => - stderr.includes(fragment), - ); -} - -function isInvalidRevsetMessage(stderr: string) { - return [ - "Failed to parse revset", - "Revision not found", - "No such revision", - "doesn't exist", - "is ambiguous", - "Revset expression resolved to no revisions", - ].some((fragment) => stderr.includes(fragment)); -} - -function createMissingJjExecutableError(input: JjBackedInput, jjExecutable: string) { - return new DunkUserError( - `Jujutsu is required for \`${formatJjCommandLabel(input)}\` when \`vcs = "jj"\`, but \`${jjExecutable}\` was not found in PATH.`, - ['Install Jujutsu or set `vcs = "git"` in dunk config, then try again.'], - ); -} - -function createMissingJjRepoError(input: JjBackedInput) { - return new DunkUserError( - `\`${formatJjCommandLabel(input)}\` must be run inside a Jujutsu repository when \`vcs = "jj"\`.`, - ['Run the command from a Jujutsu checkout, or set `vcs = "git"` in dunk config.'], - ); -} - -export function createJjStagedError(input: VcsCommandInput) { - return new DunkUserError( - `\`${formatJjCommandLabel(input)}\` requires Git VCS mode because Jujutsu has no staging area.`, - ['Remove `--staged`, or set `vcs = "git"` in dunk config.'], - ); -} - -function createInvalidRevsetError(input: JjBackedInput) { - const revset = input.kind === "vcs" ? input.range : (input.ref ?? "@"); - return new DunkUserError( - `\`${formatJjCommandLabel(input)}\` could not resolve Jujutsu revset \`${revset}\`.`, - ["Check the revset and try again."], - ); -} - -function createGenericJjError(input: JjBackedInput, stderr: string) { - return new DunkUserError(`\`${formatJjCommandLabel(input)}\` failed.`, [ - firstJjErrorLine(stderr), - ]); -} - -function translateJjSpawnFailure( - input: JjBackedInput, - error: unknown, - jjExecutable: string, -): Error { - if (error instanceof DunkUserError) { - return error; - } - - if (error instanceof Error && error.message.includes("Executable not found in $PATH")) { - return createMissingJjExecutableError(input, jjExecutable); - } - - return error instanceof Error ? error : new Error(String(error)); -} - -function translateJjExitFailure(input: JjBackedInput, stderr: string) { - if (isMissingJjRepoMessage(stderr)) { - return createMissingJjRepoError(input); - } - - if (isInvalidRevsetMessage(stderr)) { - return createInvalidRevsetError(input); - } - - return createGenericJjError(input, stderr); -} - -/** Spawn one Jujutsu command and accept only declared non-error exit codes. */ -function runJjCommand({ input, args, cwd = process.cwd(), jjExecutable = "jj" }: RunJjTextOptions) { - let proc: ReturnType; - const command = [jjExecutable, "--no-pager", "--color", "never", ...args]; - - try { - proc = Bun.spawnSync(command, { - cwd, - stdin: "ignore", - stdout: "pipe", - stderr: "pipe", - }); - } catch (error) { - throw translateJjSpawnFailure(input, error, jjExecutable); - } - - const stdout = Buffer.from(proc.stdout ?? []).toString("utf8"); - const stderr = Buffer.from(proc.stderr ?? []).toString("utf8"); - - if (proc.exitCode !== 0) { - throw translateJjExitFailure(input, stderr.trim() || `Command failed: ${command.join(" ")}`); - } - - return { - stdout, - exitCode: proc.exitCode, - }; -} - -/** Run a Jujutsu command and translate common failures into user-facing dunk errors. */ -export function runJjText(options: RunJjTextOptions) { - return runJjCommand(options).stdout; -} - -export function resolveJjRepoRoot( - input: JjBackedInput, - options: Omit = {}, -) { - return runJjText({ - input, - args: ["root"], - ...options, - }).trim(); -} diff --git a/src/core/loaders.test.ts b/src/core/loaders.test.ts index 87b38c4..1be5537 100644 --- a/src/core/loaders.test.ts +++ b/src/core/loaders.test.ts @@ -6,7 +6,6 @@ import { loadAppBootstrap } from "./loaders"; import type { CliInput } from "./types"; const tempDirs: string[] = []; -const realJjTest = Bun.which("jj") ? test : test.skip; function cleanupTempDirs() { while (tempDirs.length > 0) { @@ -39,34 +38,6 @@ function git(cwd: string, ...cmd: string[]) { return Buffer.from(proc.stdout).toString("utf8"); } -function jj(cwd: string, ...cmd: string[]) { - const proc = Bun.spawnSync( - [ - "jj", - "--config", - "signing.behavior=drop", - "--config", - 'user.name="Test User"', - "--config", - "user.email=test@example.com", - ...cmd, - ], - { - cwd, - stdout: "pipe", - stderr: "pipe", - stdin: "ignore", - }, - ); - - if (proc.exitCode !== 0) { - const stderr = Buffer.from(proc.stderr).toString("utf8"); - throw new Error(stderr.trim() || `jj ${cmd.join(" ")} failed`); - } - - return Buffer.from(proc.stdout).toString("utf8"); -} - function createTempRepo(prefix: string) { const dir = createTempDir(prefix); @@ -78,29 +49,6 @@ function createTempRepo(prefix: string) { return dir; } -function createTempJjRepo(prefix: string) { - const dir = createTempDir(prefix); - - jj(tmpdir(), "git", "init", "--colocate", dir); - - return dir; -} - -async function runWithHome(home: string, task: () => Promise) { - const previousHome = process.env.HOME; - process.env.HOME = home; - - try { - return await task(); - } finally { - if (previousHome === undefined) { - delete process.env.HOME; - } else { - process.env.HOME = previousHome; - } - } -} - async function loadFromCwd(cwd: string, input: CliInput) { const previousCwd = process.cwd(); process.chdir(cwd); @@ -608,55 +556,6 @@ describe("loadAppBootstrap", () => { expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["beta.ts"]); }); - realJjTest("loads jj diff output for a configured revset", async () => { - const home = createTempDir("hunk-jj-home-"); - - await runWithHome(home, async () => { - const dir = createTempJjRepo("hunk-jj-revset-"); - - writeFileSync(join(dir, "alpha.ts"), "export const alpha = 1;\n"); - jj(dir, "commit", "-m", "initial"); - - writeFileSync(join(dir, "alpha.ts"), "export const alpha = 2;\n"); - writeFileSync(join(dir, "beta.ts"), "export const beta = true;\n"); - - const bootstrap = await loadFromRepo(dir, { - kind: "vcs", - range: "@", - staged: false, - options: { mode: "auto", vcs: "jj" }, - }); - - expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["alpha.ts", "beta.ts"]); - expect(bootstrap.changeset.title).toStartWith("hunk-jj-revset-"); - expect(bootstrap.changeset.title).toEndWith(" @"); - }); - }); - - realJjTest("loads jj show output for a configured revset", async () => { - const home = createTempDir("hunk-jj-home-"); - - await runWithHome(home, async () => { - const dir = createTempJjRepo("hunk-jj-show-"); - - writeFileSync(join(dir, "alpha.ts"), "export const alpha = 1;\n"); - jj(dir, "commit", "-m", "initial"); - - writeFileSync(join(dir, "alpha.ts"), "export const alpha = 2;\n"); - jj(dir, "commit", "-m", "update alpha"); - - const bootstrap = await loadFromRepo(dir, { - kind: "show", - ref: "@-", - options: { mode: "auto", vcs: "jj" }, - }); - - expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["alpha.ts"]); - expect(bootstrap.changeset.title).toStartWith("hunk-jj-show-"); - expect(bootstrap.changeset.title).toEndWith(" show @-"); - }); - }); - test("applies pathspec filtering to untracked files in working tree reviews", async () => { const dir = createTempRepo("hunk-git-untracked-pathspec-"); @@ -804,17 +703,6 @@ describe("loadAppBootstrap", () => { expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["alpha.ts"]); }); - test("rejects stash show when configured for jj", async () => { - const dir = createTempDir("hunk-stash-jj-"); - - await expect( - loadFromRepo(dir, { - kind: "stash-show", - options: { mode: "auto", vcs: "jj" }, - }), - ).rejects.toThrow("`dunk stash show` requires Git VCS mode."); - }); - test("reports a friendly error when no stash entries exist", async () => { const dir = createTempRepo("hunk-stash-empty-"); diff --git a/src/core/loaders.ts b/src/core/loaders.ts index ecd840c..99f3533 100644 --- a/src/core/loaders.ts +++ b/src/core/loaders.ts @@ -17,10 +17,9 @@ import { } from "./comments"; import { LARGE_FILE_MAX_BYTES } from "./limits"; import { DEFAULT_VIEW_PREFERENCES, findRepoRoot } from "./config"; -import { resolveGitBranchBase, resolveJjBranchBase } from "./branchReview"; +import { resolveGitBranchBase } from "./branchReview"; import type { DriftedCommentSummary } from "./types"; import { normalizeDiffMetadataPaths, normalizeDiffPath } from "./diffPaths"; -import { DunkUserError } from "./errors"; import { buildGitDiffArgs, buildGitDiffNumstatArgs, @@ -31,13 +30,6 @@ import { runGitText, runGitUntrackedFileDiffText, } from "./git"; -import { - buildJjDiffArgs, - buildJjShowArgs, - createJjStagedError, - resolveJjRepoRoot, - runJjText, -} from "./jj"; import type { AppBootstrap, Changeset, @@ -984,37 +976,6 @@ async function loadGitChangeset( }; } -/** Build a changeset from the current Jujutsu working-copy commit or a revset. */ -async function loadJjDiffChangeset( - input: VcsCommandInput, - cwd = process.cwd(), -): Promise { - if (input.staged) { - throw createJjStagedError(input); - } - - const repoRoot = resolveJjRepoRoot(input, { cwd }); - const repoName = basename(repoRoot); - - const branchBase = input.branchReview ? resolveJjBranchBase(input) : undefined; - const title = branchBase - ? `${repoName} branch vs ${branchBase.displayBase}` - : input.range - ? `${repoName} ${input.range}` - : `${repoName} working copy`; - - const changeset = normalizePatchChangeset( - runJjText({ input, args: buildJjDiffArgs(input, branchBase?.jjFromRevset), cwd }), - title, - repoRoot, - ); - - return { - changeset, - sessionNotice: branchBase ? `branch base: ${branchBase.displayBase}` : undefined, - }; -} - /** Build a changeset from `git show`, suppressing commit-message chrome so only the patch feeds the UI. */ async function loadShowChangeset(input: ShowCommandInput, cwd = process.cwd()) { const repoRoot = resolveGitRepoRoot(input, { cwd }); @@ -1027,27 +988,8 @@ async function loadShowChangeset(input: ShowCommandInput, cwd = process.cwd()) { ); } -/** Build a changeset from one Jujutsu revset using Git-format patch output. */ -async function loadJjShowChangeset(input: ShowCommandInput, cwd = process.cwd()) { - const repoRoot = resolveJjRepoRoot(input, { cwd }); - const repoName = basename(repoRoot); - const revset = input.ref ?? "@"; - - return normalizePatchChangeset( - runJjText({ input, args: buildJjShowArgs(input), cwd }), - `${repoName} show ${revset}`, - repoRoot, - ); -} - /** Build a changeset from `git stash show -p`, which naturally maps to one reviewable patch. */ async function loadStashShowChangeset(input: StashShowCommandInput, cwd = process.cwd()) { - if (input.options.vcs === "jj") { - throw new DunkUserError("`dunk stash show` requires Git VCS mode.", [ - 'Set `vcs = "git"` in dunk config, then try again.', - ]); - } - const repoRoot = resolveGitRepoRoot(input, { cwd }); const repoName = basename(repoRoot); @@ -1080,19 +1022,13 @@ export async function loadAppBootstrap( switch (input.kind) { case "vcs": { - const loaded = - input.options.vcs === "jj" - ? await loadJjDiffChangeset(input, cwd) - : await loadGitChangeset(input, cwd); + const loaded = await loadGitChangeset(input, cwd); changeset = loaded.changeset; sessionNotice = loaded.sessionNotice; break; } case "show": - changeset = - input.options.vcs === "jj" - ? await loadJjShowChangeset(input, cwd) - : await loadShowChangeset(input, cwd); + changeset = await loadShowChangeset(input, cwd); break; case "stash-show": changeset = await loadStashShowChangeset(input, cwd); diff --git a/src/core/types.ts b/src/core/types.ts index 1f2af43..cb49b79 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -1,7 +1,6 @@ import type { FileDiffMetadata } from "@pierre/diffs"; export type LayoutMode = "auto" | "split" | "stack"; -export type VcsMode = "git" | "jj"; /** Inclusive 1-based [start, end] line range used by annotations and persisted comments. */ export type LineRange = [number, number]; @@ -43,7 +42,6 @@ export interface Changeset { export interface CommonOptions { mode?: LayoutMode; - vcs?: VcsMode; theme?: string; pager?: boolean; watch?: boolean; diff --git a/src/core/watch.ts b/src/core/watch.ts index 978c79e..50ba62d 100644 --- a/src/core/watch.ts +++ b/src/core/watch.ts @@ -3,7 +3,6 @@ import { join } from "node:path"; import { findRepoRoot } from "./config"; import { DUNK_COMMENTS_RELATIVE_PATH } from "./dunkPaths"; import { buildGitDiffRawArgs, listGitUntrackedFiles, resolveGitRepoRoot, runGitText } from "./git"; -import { runJjText } from "./jj"; import type { CliInput } from "./types"; /** Return whether the current input can be rebuilt from files or VCS state without rereading stdin. */ @@ -32,8 +31,7 @@ function statSignature(path: string) { * * Polling should scale with filesystem activity, not patch size. Use cheap * commands that only report whether something changed: `git diff --raw` for - * tracked work, `git rev-parse` for ref-backed reviews, and - * `jj log -T commit_id` for Jujutsu inputs. + * tracked work and `git rev-parse` for ref-backed reviews. * * `--raw` rather than `--numstat`: numstat reports only counts, so two * same-shape edits (foo -> bar then bar -> baz, each "1\t1\tpath") would share a @@ -66,34 +64,13 @@ function gitVcsSignature(input: Extract) { - // jj log with a fixed template emits just the commit id, which is enough - // to detect any change in the working copy or the reviewed revset. - switch (input.kind) { - case "vcs": - return runJjText({ - input, - args: ["log", "--no-graph", "-T", "commit_id", "-r", input.range ?? "@"], - }); - case "show": - return runJjText({ - input, - args: ["log", "--no-graph", "-T", "commit_id", "-r", input.ref ?? "@"], - }); - } -} - /** Compute a change-detection signature for one watchable input. */ export function computeWatchSignature(input: CliInput) { const parts: string[] = [input.kind]; switch (input.kind) { case "vcs": - parts.push(input.options.vcs === "jj" ? jjVcsSignature(input) : gitVcsSignature(input)); - break; case "show": - parts.push(input.options.vcs === "jj" ? jjVcsSignature(input) : gitVcsSignature(input)); - break; case "stash-show": parts.push(gitVcsSignature(input)); break;