diff --git a/e2e/conflictMarkers.spec.ts b/e2e/conflictMarkers.spec.ts new file mode 100644 index 000000000..4d56a012f --- /dev/null +++ b/e2e/conflictMarkers.spec.ts @@ -0,0 +1,22 @@ +import { expect, openFromTree, test } from "./fixtures"; + +const MARKERS = ["<<<<<<< Updated upstream", "ours();", "=======", "theirs();", ">>>>>>> Stashed changes"]; + +test("flags merge conflict markers typed into a file as errors, with no language server", async ({ + workbench: page, +}) => { + const editor = await openFromTree(page, "build.gradle.kts"); + await editor.locator(".cm-content").click(); + await page.keyboard.press("Meta+ArrowDown"); + await page.keyboard.press("Enter"); + // Typed line by line: the editor indents each new line like the one above. + for (const line of MARKERS) { + await page.keyboard.type(line); + await page.keyboard.press("Enter"); + } + // One error on each of the three marker lines. + await expect(editor.locator(".cm-lintRange-error")).toHaveCount(3); + // Undoing the marker lines clears them again. + for (let i = 0; i < 40; i++) await page.keyboard.press("Meta+z"); + await expect(editor.locator(".cm-lintRange-error")).toHaveCount(0); +}); diff --git a/src/features/settings/__tests__/inspections.test.ts b/src/features/settings/__tests__/inspections.test.ts index 32a134972..2725a2dcd 100644 --- a/src/features/settings/__tests__/inspections.test.ts +++ b/src/features/settings/__tests__/inspections.test.ts @@ -53,6 +53,7 @@ describe("inspections tree", () => { "Python", "SQL", "TypeScript", + "Version control", ]); const python = find(tree, "group:Python")!; expect(python.children.map((n) => n.id)).toEqual(["lsp:basedpyright", "lsp:ruff"]); diff --git a/src/features/settings/pages/editor/inspections.tsx b/src/features/settings/pages/editor/inspections.tsx index ce2f8df01..904aca388 100644 --- a/src/features/settings/pages/editor/inspections.tsx +++ b/src/features/settings/pages/editor/inspections.tsx @@ -418,7 +418,7 @@ function InspectionRow({ return ( - {node.name} + {node.name} {levelLabel && {levelLabel}} @@ -454,7 +454,9 @@ function Details({ const id = node.id; return (
-

{node.description}

+

+ {node.description} +

{id && (
diff --git a/src/features/vcs/BranchWidget.tsx b/src/features/vcs/BranchWidget.tsx index 534e5f4d7..3ce432b2b 100644 --- a/src/features/vcs/BranchWidget.tsx +++ b/src/features/vcs/BranchWidget.tsx @@ -232,9 +232,11 @@ function OptionsButton({ hasTags }: { hasTags: boolean }) { /** Entries IDEA puts at the top of the Branches popup while an operation is stopped. */ export function operationActions(op: InProgress | null, conflicts: number): PopupAction[] { - if (!op) return []; const out: PopupAction[] = []; + // Conflicts are resolved whether or not an operation is stopped: applying a stash, or the + // autostash of a pull, leaves them with none in progress. if (conflicts) out.push({ id: "vcs.resolveConflicts", label: "Resolve Conflicts…", icon: "warning" }); + if (!op) return out; const ids: Record> = { rebase: [ ["vcs.rebase.continue", "Continue Rebase"], diff --git a/src/features/vcs/MergeRebaseWidget.module.css b/src/features/vcs/MergeRebaseWidget.module.css new file mode 100644 index 000000000..9d376c971 --- /dev/null +++ b/src/features/vcs/MergeRebaseWidget.module.css @@ -0,0 +1,29 @@ +.widget { + display: inline-flex; + align-items: center; + gap: 6px; +} +.go, +.abort { + --icon-color: var(--fg-on-accent); + display: inline-flex; + align-items: center; + justify-content: center; + width: 30px; + height: 30px; + padding: 0; + border: 0; + border-radius: 6px; + cursor: pointer; +} +.go { + background: var(--success); +} +.abort { + background: var(--danger); +} +.go:disabled, +.abort:disabled { + opacity: 0.5; + cursor: default; +} diff --git a/src/features/vcs/MergeRebaseWidget.tsx b/src/features/vcs/MergeRebaseWidget.tsx new file mode 100644 index 000000000..6adbc59e7 --- /dev/null +++ b/src/features/vcs/MergeRebaseWidget.tsx @@ -0,0 +1,64 @@ +/** + * Title bar buttons for a merge, rebase, cherry-pick or revert that stopped, and for conflicts + * left by anything else (IntelliJ's GitMergeRebaseWidget): Resolve and Continue green, Abort red. + */ +import { executeAction } from "@/platform/actions"; +import { tx } from "@/platform/i18n"; +import { Icon } from "@/ui"; +import type { IconName } from "@/ui/icons"; +import { controlLabel, controlOperation, inProgress } from "./operations"; +import { useVcs } from "./store"; +import styles from "./MergeRebaseWidget.module.css"; + +function Button(props: { label: string; icon: IconName; tone: "go" | "abort"; disabled: boolean; run: () => void }) { + return ( + + ); +} + +export function MergeRebaseWidget() { + const op = useVcs((s) => inProgress(s.branch?.operation)); + const conflicts = useVcs((s) => s.status.some((f) => f.unstaged === "conflicted")); + const busy = useVcs((s) => !!s.remoteBusy); + if (!op && !conflicts) return null; + return ( +
+ {conflicts && ( +
+ ); +} diff --git a/src/features/vcs/__tests__/conflictBanner.test.tsx b/src/features/vcs/__tests__/conflictBanner.test.tsx new file mode 100644 index 000000000..f0c75d5bf --- /dev/null +++ b/src/features/vcs/__tests__/conflictBanner.test.tsx @@ -0,0 +1,47 @@ +import { act, cleanup, renderHook } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; +import { useEditorBanners } from "@/workbench/contributions"; +import { activate } from "../conflictBanner"; +import { useVcs } from "../store"; + +function status(path: string, unstaged: "conflicted" | "modified") { + const file = { path, staged: null, unstaged }; + useVcs.setState({ status: [file as never], byPath: new Map([[path, file as never]]) }); +} + +describe("the conflict banner", () => { + let dispose = () => {}; + afterEach(() => { + // Unmount first: a mounted hook would render again once the banner is gone. + cleanup(); + dispose(); + useVcs.setState({ status: [], byPath: new Map() }); + }); + + const contribution = () => { + dispose = activate(); + return useEditorBanners.getState().banners.get("vcs.conflict")!; + }; + + it("warns above a file Git records as conflicted, and offers to resolve it", () => { + const banner = contribution(); + status("/p/settings.gradle", "conflicted"); + const { result } = renderHook(() => banner.useBanner("/p/settings.gradle")); + expect(result.current).toMatchObject({ + level: "warning", + text: "File has unresolved merge conflicts", + actions: [{ label: "Resolve conflicts…" }], + }); + }); + + it("shows nothing above other files, and goes away when the conflict is resolved", () => { + const banner = contribution(); + status("/p/settings.gradle", "conflicted"); + const other = renderHook(() => banner.useBanner("/p/other.gradle")); + expect(other.result.current).toBeNull(); + const { result } = renderHook(() => banner.useBanner("/p/settings.gradle")); + expect(result.current).not.toBeNull(); + act(() => status("/p/settings.gradle", "modified")); + expect(result.current).toBeNull(); + }); +}); diff --git a/src/features/vcs/__tests__/conflictMarkerEdits.test.ts b/src/features/vcs/__tests__/conflictMarkerEdits.test.ts new file mode 100644 index 000000000..10a1e2cba --- /dev/null +++ b/src/features/vcs/__tests__/conflictMarkerEdits.test.ts @@ -0,0 +1,38 @@ +import { EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { expect, it, vi } from "vitest"; +import { useEditorRegistry } from "@/editor/api"; +import { setFileDiagnostics } from "@/editor/lsp/diagnostics"; +import { activate } from "../conflictMarkers"; + +vi.mock("@/editor/lsp/diagnostics", () => ({ setFileDiagnostics: vi.fn() })); + +it("coalesces a burst of edits in a large document into one inspection", () => { + vi.useFakeTimers(); + const dispose = activate(); + const extensions = useEditorRegistry.getState().contributions.get("merge-conflict-markers")!.extensions({ + doc: 1, + path: "/large.md", + language: "markdown", + uiMode: null, + readOnly: false, + }); + const view = new EditorView({ + state: EditorState.create({ doc: "plain text\n".repeat(100_000), extensions }), + }); + try { + vi.mocked(setFileDiagnostics).mockClear(); + for (const insert of ["<<<<<<< ours\n", "a\n=======\n", "b\n>>>>>>> theirs\n"]) { + view.dispatch({ changes: { from: view.state.doc.length, insert } }); + vi.advanceTimersByTime(100); + } + expect(setFileDiagnostics).not.toHaveBeenCalled(); + vi.advanceTimersByTime(250); + expect(setFileDiagnostics).toHaveBeenCalledTimes(1); + expect(vi.mocked(setFileDiagnostics).mock.calls[0]![2]).toHaveLength(3); + } finally { + view.destroy(); + dispose(); + vi.useRealTimers(); + } +}); diff --git a/src/features/vcs/__tests__/conflictMarkers.test.ts b/src/features/vcs/__tests__/conflictMarkers.test.ts new file mode 100644 index 000000000..70c5198e3 --- /dev/null +++ b/src/features/vcs/__tests__/conflictMarkers.test.ts @@ -0,0 +1,44 @@ +import { Text } from "@codemirror/state"; +import { describe, expect, it, vi } from "vitest"; +import { markerDiagnostics } from "../conflictMarkers"; + +const doc = (s: string) => Text.of(s.split("\n")); + +describe("markerDiagnostics", () => { + it("reports an error on every marker line of a conflict, whatever the language", () => { + const text = "def allServices = [\n<<<<<<< Updated upstream\n 'a',\n=======\n 'b',\n>>>>>>> Stashed changes\n]\n"; + const found = markerDiagnostics(doc(text)); + expect(found.map((d) => [d.range.start.line, d.range.end.line, d.severity])).toEqual([ + [1, 1, "error"], + [3, 3, "error"], + [5, 5, "error"], + ]); + expect(found[0]!.range.end.col).toBe("<<<<<<< Updated upstream".length); + expect(found.every((d) => d.message === "Unresolved merge conflict marker" && d.source === "Git")).toBe(true); + }); + + it("includes the base marker of a diff3 conflict", () => { + const text = "<<<<<<< a\nx\n||||||| base\nb\n=======\ny\n>>>>>>> c\n"; + expect(markerDiagnostics(doc(text))).toHaveLength(4); + }); + + it("uses UTF-16 columns and excludes CRLF terminators", () => { + const found = markerDiagnostics(doc("😀\r\n<<<<<<< ours 😀\r\nx\r\n=======\r\ny\r\n>>>>>>> theirs\r\n")); + expect(found[0]!.range).toEqual({ start: { line: 1, col: 0 }, end: { line: 1, col: 15 } }); + expect(found[1]!.range).toEqual({ start: { line: 3, col: 0 }, end: { line: 3, col: 7 } }); + }); + + it("skips oversized documents without flattening their text", () => { + const large = doc("x".repeat(2_000_001)); + const flatten = vi.spyOn(large, "toString"); + expect(markerDiagnostics(large)).toEqual([]); + expect(flatten).not.toHaveBeenCalled(); + }); + + it("reports nothing for text without conflicts, or a block that was never closed", () => { + expect(markerDiagnostics(doc("a\nb\n"))).toEqual([]); + expect(markerDiagnostics(doc("<<<<<<< a\nx\n=======\ny\n"))).toEqual([]); + // A heading underline of seven equals signs is not a conflict. + expect(markerDiagnostics(doc("Title\n=======\ntext\n"))).toEqual([]); + }); +}); diff --git a/src/features/vcs/__tests__/conflicts.test.ts b/src/features/vcs/__tests__/conflicts.test.ts index 12b6fddea..b04ee6357 100644 --- a/src/features/vcs/__tests__/conflicts.test.ts +++ b/src/features/vcs/__tests__/conflicts.test.ts @@ -36,6 +36,19 @@ describe("parseConflicts", () => { expect(blockAt(blocks, 0)).toBeUndefined(); }); + it("locates the marker lines, without their terminators", () => { + const [a, b] = parseConflicts(text); + const lines = (block: typeof a) => block!.markers.map((m) => text.slice(m.from, m.to)); + expect(lines(a)).toEqual(["<<<<<<< HEAD", "||||||| base", "=======", ">>>>>>> feature"]); + expect(lines(b)).toEqual(["<<<<<<< HEAD", "=======", ">>>>>>> feature"]); + const crlf = "<<<<<<< a\r\nx\r\n=======\r\ny\r\n>>>>>>> b\r\n"; + expect(parseConflicts(crlf)[0]!.markers.map((m) => crlf.slice(m.from, m.to))).toEqual([ + "<<<<<<< a", + "=======", + ">>>>>>> b", + ]); + }); + it("ignores unterminated markers", () => { expect(parseConflicts("<<<<<<< HEAD\nx\n=======\ny\n")).toEqual([]); }); diff --git a/src/features/vcs/__tests__/historyOps.test.ts b/src/features/vcs/__tests__/historyOps.test.ts index 87427edf8..bad25f9cb 100644 --- a/src/features/vcs/__tests__/historyOps.test.ts +++ b/src/features/vcs/__tests__/historyOps.test.ts @@ -47,6 +47,11 @@ describe("operations in progress", () => { ]); expect(operationActions("merge", 0).map((a) => a.id)).toEqual(["vcs.merge.abort"]); }); + + it("offers Resolve Conflicts when conflicts remain with no operation in progress", () => { + // Applying a stash, or the autostash of a pull, leaves conflicts and no merge or rebase. + expect(operationActions(null, 1).map((a) => a.id)).toEqual(["vcs.resolveConflicts"]); + }); }); describe("Git Log commit actions", () => { diff --git a/src/features/vcs/__tests__/mergeRebaseWidget.test.tsx b/src/features/vcs/__tests__/mergeRebaseWidget.test.tsx new file mode 100644 index 000000000..58e0ecd84 --- /dev/null +++ b/src/features/vcs/__tests__/mergeRebaseWidget.test.tsx @@ -0,0 +1,43 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { cleanup, render, screen } from "@testing-library/react"; +import { setLocale } from "@/platform/i18n"; +import { MergeRebaseWidget } from "../MergeRebaseWidget"; +import { useVcs } from "../store"; + +describe("MergeRebaseWidget", () => { + afterEach(() => { + cleanup(); + useVcs.setState({ status: [], branch: null }); + }); + + it("renders nothing without conflicts or an operation", () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + + it("offers only Resolve for conflicts left without an operation (stash pop)", () => { + useVcs.setState({ status: [{ path: "a", unstaged: "conflicted" } as never] }); + render(); + expect(screen.getByLabelText("Resolve Conflicts…")).toBeTruthy(); + expect(screen.queryByLabelText(/Abort/)).toBeNull(); + }); + + it("offers Continue and Abort for a rebase with nothing left to resolve", () => { + useVcs.setState({ branch: { head: "x", operation: "rebase" } as never }); + render(); + expect(screen.getByLabelText("Continue Rebase")).toBeTruthy(); + expect(screen.getByLabelText("Abort Rebase")).toBeTruthy(); + }); + + it("translates operation button labels", async () => { + await setLocale("zh"); + try { + useVcs.setState({ branch: { head: "x", operation: "rebase" } as never }); + render(); + expect(screen.getByLabelText("继续变基")).toHaveAttribute("title", "继续变基"); + expect(screen.getByLabelText("中止变基")).toHaveAttribute("title", "中止变基"); + } finally { + await setLocale("en"); + } + }); +}); diff --git a/src/features/vcs/conflictBanner.ts b/src/features/vcs/conflictBanner.ts new file mode 100644 index 000000000..218ca9448 --- /dev/null +++ b/src/features/vcs/conflictBanner.ts @@ -0,0 +1,30 @@ +/** + * The notice above a file that has unresolved merge conflicts, as IntelliJ shows it + * ("File has unresolved merge conflicts" with Resolve conflicts…). It follows Git's own + * record of the conflict, so it appears whatever the file's text holds and whether or not + * a merge, rebase or stash is still in progress. + */ +import { registerEditorBanner } from "@/workbench/contributions"; +import { openMergeEditor } from "./conflictsUi"; +import { effectiveKind, stripSlash } from "./model"; +import { useVcs } from "./store"; + +export function activate(): () => void { + return registerEditorBanner({ + id: "vcs.conflict", + order: -5, + useBanner: (path) => { + const conflicted = useVcs((s) => { + const file = s.byPath.get(stripSlash(path)); + return !!file && effectiveKind(file) === "conflicted"; + }); + return conflicted + ? { + text: "File has unresolved merge conflicts", + level: "warning", + actions: [{ label: "Resolve conflicts…", run: () => openMergeEditor(path) }], + } + : null; + }, + }); +} diff --git a/src/features/vcs/conflictMarkers.ts b/src/features/vcs/conflictMarkers.ts new file mode 100644 index 000000000..e358cf028 --- /dev/null +++ b/src/features/vcs/conflictMarkers.ts @@ -0,0 +1,70 @@ +/** + * Reports the conflict markers a merge left in a file as errors, whatever its language and + * whether or not a language server runs for it: IntelliJ's own parsers fail on them, but + * without a server (Groovy, plain text, …) nothing else would say the file is unresolved. + */ +import { ViewPlugin } from "@codemirror/view"; +import type { Text } from "@codemirror/state"; +import { registerEditorContribution } from "@/editor/api"; +import { setFileDiagnostics } from "@/editor/lsp/diagnostics"; +import { offsetsToRange } from "@/editor/lsp/positions"; +import type { Diagnostic } from "@/bindings"; +import { tx } from "@/platform/i18n"; +import { CONFLICT_MARKERS_PRODUCER } from "@/platform/inspections"; +import { parseConflicts } from "./conflicts"; + +/** Large files are not scanned on each edit. */ +const MAX_LENGTH = 2_000_000; +const DELAY_MS = 250; + +/** An error on every marker line of the complete conflict blocks in `doc`. */ +export function markerDiagnostics(doc: Text): Diagnostic[] { + if (doc.length > MAX_LENGTH) return []; + const text = doc.toString(); + // A cheap test first: most files have no marker at all. + if (!text.includes("<<<<<<<")) return []; + return parseConflicts(text).flatMap((block) => + block.markers.map((m) => ({ + range: offsetsToRange(doc, m.from, m.to), + severity: "error" as const, + message: tx("Unresolved merge conflict marker"), + source: "Git", + code: null, + })), + ); +} + +/** How many editors show each file: its diagnostics go away with the last one. */ +const viewers = new Map(); + +export function activate(): () => void { + return registerEditorContribution({ + id: "merge-conflict-markers", + extensions: (doc) => + doc.virtual + ? [] + : ViewPlugin.define((view) => { + viewers.set(doc.path, (viewers.get(doc.path) ?? 0) + 1); + let timer: ReturnType | undefined; + const publish = () => + setFileDiagnostics(doc.path, CONFLICT_MARKERS_PRODUCER, markerDiagnostics(view.state.doc)); + publish(); + return { + update: (u) => { + if (!u.docChanged) return; + clearTimeout(timer); + timer = setTimeout(publish, DELAY_MS); + }, + destroy: () => { + clearTimeout(timer); + const remaining = (viewers.get(doc.path) ?? 1) - 1; + if (remaining) viewers.set(doc.path, remaining); + else { + viewers.delete(doc.path); + setFileDiagnostics(doc.path, CONFLICT_MARKERS_PRODUCER, []); + } + }, + }; + }), + }); +} diff --git a/src/features/vcs/conflicts.ts b/src/features/vcs/conflicts.ts index 1a5179842..e1a1211c5 100644 --- a/src/features/vcs/conflicts.ts +++ b/src/features/vcs/conflicts.ts @@ -17,6 +17,8 @@ export interface ConflictBlock { oursTo: number; theirsFrom: number; theirsTo: number; + /** The marker lines (`<<<<<<<`, `|||||||` when there is a base, `=======`, `>>>>>>>`), without their terminators. */ + markers: Array<{ from: number; to: number }>; } const START = "<<<<<<<"; @@ -70,6 +72,9 @@ export function parseConflicts(text: string): ConflictBlock[] { oursTo: lines[oursEnd]!.from, theirsFrom: lines[sepAt]!.end, theirsTo: lines[endAt]!.from, + markers: [i, baseAt, sepAt, endAt] + .filter((j) => j >= 0) + .map((j) => ({ from: lines[j]!.from, to: lines[j]!.from + lines[j]!.text.replace(/\r?\n$/, "").length })), }); i = endAt; } diff --git a/src/features/vcs/index.ts b/src/features/vcs/index.ts index b360573d9..949f5651c 100644 --- a/src/features/vcs/index.ts +++ b/src/features/vcs/index.ts @@ -38,12 +38,43 @@ import { registerMergeActions } from "./mergeActions"; import { baseName } from "./model"; import { installDiagnosticsTracking } from "./commitChecks"; import { installPreparedMessage, useCommit } from "./commitStore"; +import { activate as activateConflictBanner } from "./conflictBanner"; +import { activate as activateConflictMarkers } from "./conflictMarkers"; +import { MergeRebaseWidget } from "./MergeRebaseWidget"; import { installFileConfirmations } from "./confirmations"; import { installIncomingCheck } from "./incoming"; import { installGitLog, selectGitTab, useGitLog } from "./logStore"; import { installVcsStore, onConflictsAppeared, onConflictsCleared, useVcs } from "./store"; import { tx } from "@/platform/i18n"; +/** The Resolve/Continue/Abort buttons exist in the title bar only while there is something to resolve. */ +function trackMergeRebaseItem(): () => void { + let dispose: (() => void) | null = null; + const sync = () => { + const s = useVcs.getState(); + const needed = + s.available && (!!inProgress(s.branch?.operation) || s.status.some((f) => f.unstaged === "conflicted")); + if (needed && !dispose) { + dispose = registerTitleItem({ + id: "vcs.mergeRebase", + title: "Merge/Rebase Widget", + align: "left", + priority: 11, + component: MergeRebaseWidget, + }); + } else if (!needed && dispose) { + dispose(); + dispose = null; + } + }; + sync(); + const unsub = useVcs.subscribe(sync); + return () => { + unsub(); + dispose?.(); + }; +} + function trackTitleItem(): () => void { let dispose: (() => void) | null = null; const sync = (available: boolean) => { @@ -182,8 +213,11 @@ export function activate(): () => void { registerCompareActions(), registerMergeActions(), trackTitleItem(), + trackMergeRebaseItem(), installHunkPopup(), registerOverlay({ id: "vcs.dialogs", component: VcsDialogHost }), + activateConflictBanner(), + activateConflictMarkers(), trackCommitWindow(), registerToolWindow({ id: "git", diff --git a/src/features/vcs/operations.tsx b/src/features/vcs/operations.tsx index b57298b47..d0a9bbe0b 100644 --- a/src/features/vcs/operations.tsx +++ b/src/features/vcs/operations.tsx @@ -179,24 +179,22 @@ export function OperationBanner() { const conflicts = useVcs((s) => s.status.filter((f) => f.unstaged === "conflicted").length); const busy = useVcs((s) => s.remoteBusy); const op = inProgress(branch?.operation); - if (!op) return null; + // Conflicts need a notice with no operation in progress too (applying a stash leaves them). + if (!op && !conflicts) return null; + const count = conflicts === 1 ? tx`${conflicts} conflict` : tx`${conflicts} conflicts`; return ( -
+
- {operationTitle(op, branch?.head ?? null)} - {conflicts - ? ` · ${conflicts === 1 ? tx`${conflicts} conflict` : tx`${conflicts} conflicts`}` - : op === "merge" - ? tx(" · commit to conclude") - : ""} + {op ? operationTitle(op, branch?.head ?? null) : tx("Unresolved merge conflicts")} + {conflicts ? ` · ${count}` : op === "merge" ? tx(" · commit to conclude") : ""} {conflicts > 0 && ( )} - {operationControls(op).map((c) => ( + {(op ? operationControls(op) : []).map((c) => (