diff --git a/packages/opencode/src/file/index.ts b/packages/opencode/src/file/index.ts index 32465015e9c..1b5e37850bf 100644 --- a/packages/opencode/src/file/index.ts +++ b/packages/opencode/src/file/index.ts @@ -263,6 +263,16 @@ export namespace File { "file.edited", z.object({ file: z.string(), + changedRanges: z + .array( + z.object({ + start: z.number(), + end: z.number(), + _byteOffset: z.number().optional(), + _byteLength: z.number().optional(), + }), + ) + .optional(), }), ), } diff --git a/packages/opencode/src/format/diff-range.ts b/packages/opencode/src/format/diff-range.ts new file mode 100644 index 00000000000..97cd3b73329 --- /dev/null +++ b/packages/opencode/src/format/diff-range.ts @@ -0,0 +1,161 @@ +import { diffLines } from "diff" + +// Configuration constants +const MERGE_THRESHOLD = 6 // characters - about 1 line gap +const EOF_CLAMP_OFFSET = 1 // Safety margin for EOF clamping + +/** + * Clamp offset to be within valid file bounds + */ +function clampOffset(offset: number, total: number): number { + return total > 0 ? Math.min(offset, total - EOF_CLAMP_OFFSET) : 0 +} + +/** + * Simplified range representation with conversion methods + */ +export class DiffRange { + constructor( + public readonly start: number, + public readonly end: number, + ) {} + + /** + * Convert to plain object for serialization + */ + toJSON(): { start: number; end: number; _byteOffset?: number; _byteLength?: number } { + const byteOffsets = this.getCachedByteOffsets() + return { + start: this.start, + end: this.end, + _byteOffset: byteOffsets?.start, + _byteLength: byteOffsets ? byteOffsets.end - byteOffsets.start : undefined, + } + } + + /** + * Create from plain object (for deserialization) + */ + static fromJSON(data: { start: number; end: number; _byteOffset?: number; _byteLength?: number }): DiffRange { + if (data.start < 0 || data.end < 0) { + throw new Error(`Invalid range: negative offsets (start=${data.start}, end=${data.end})`) + } + if (data.start > data.end) { + throw new Error(`Invalid range: start ${data.start} > end ${data.end}`) + } + const range = new DiffRange(data.start, data.end) + if (data._byteOffset !== undefined && data._byteLength !== undefined) { + range._byteOffset = data._byteOffset + range._byteLength = data._byteLength + } + return range + } + + get length(): number { + return this.end - this.start + } + + // Private storage for cached byte offsets (mutable for factory methods) + private _byteOffset?: number + private _byteLength?: number + + /** + * Create DiffRange from character and byte offsets + */ + static fromOffsets(charOffset: number, charLength: number, byteOffset: number, byteLength: number): DiffRange { + const range = new DiffRange(charOffset, charOffset + charLength) + range._byteOffset = byteOffset + range._byteLength = byteLength + return range + } + + getCachedByteOffsets(): { start: number; end: number } | undefined { + if (this._byteOffset !== undefined && this._byteLength !== undefined) { + return { start: this._byteOffset, end: this._byteOffset + this._byteLength } + } + return undefined + } + + /** + * Check if this range should be merged with another + */ + shouldMerge(other: DiffRange): boolean { + return other.start - this.end <= MERGE_THRESHOLD + } + + /** + * Merge this range with another + */ + merge(other: DiffRange): DiffRange { + const newStart = Math.min(this.start, other.start) + const newEnd = Math.max(this.end, other.end) + const merged = new DiffRange(newStart, newEnd) + + // Merge cached byte offsets if available + const thisBytes = this.getCachedByteOffsets() + const otherBytes = other.getCachedByteOffsets() + if (thisBytes && otherBytes) { + merged._byteOffset = Math.min(thisBytes.start, otherBytes.start) + merged._byteLength = Math.max(thisBytes.end, otherBytes.end) - merged._byteOffset + } + + return merged + } +} + +/** + * Calculate changed ranges from old and new content + * Returns ranges that can be converted to both character and byte offsets + */ +export function calculateRanges(oldContent: string, newContent: string): DiffRange[] { + const changes = diffLines(oldContent.replace(/\r\n/g, "\n"), newContent.replace(/\r\n/g, "\n")) + const result: DiffRange[] = [] + + let charOffset = 0 + let byteOffset = 0 + + const totalChars = newContent.length + const totalBytes = Buffer.byteLength(newContent) + + for (const change of changes) { + if (change.added) { + const text = change.value + const bytes = Buffer.byteLength(text) + result.push(DiffRange.fromOffsets(charOffset, text.length, byteOffset, bytes)) + charOffset += text.length + byteOffset += bytes + continue + } + if (change.removed) { + result.push(DiffRange.fromOffsets(clampOffset(charOffset, totalChars), 0, clampOffset(byteOffset, totalBytes), 0)) + continue + } + const text = change.value + const bytes = Buffer.byteLength(text) + charOffset += text.length + byteOffset += bytes + } + + return mergeRanges(result) +} + +/** + * Merge adjacent ranges to reduce formatter invocations + */ +function mergeRanges(ranges: DiffRange[]): DiffRange[] { + if (ranges.length === 0) return ranges + + return ranges + .toSorted((a, b) => a.start - b.start) + .reduce((acc, cur) => { + if (acc.length === 0) return [cur] + + const last = acc[acc.length - 1] + if (last.shouldMerge(cur)) { + acc[acc.length - 1] = last.merge(cur) + return acc + } + acc.push(cur) + return acc + }, [] as DiffRange[]) +} diff --git a/packages/opencode/src/format/formatter.ts b/packages/opencode/src/format/formatter.ts index 9e97fae9dfc..cf477304fb3 100644 --- a/packages/opencode/src/format/formatter.ts +++ b/packages/opencode/src/format/formatter.ts @@ -3,6 +3,7 @@ import { BunProc } from "../bun" import { Instance } from "../project/instance" import { Filesystem } from "../util/filesystem" import { Flag } from "@/flag/flag" +import type { DiffRange } from "./diff-range" export interface Info { name: string @@ -10,6 +11,7 @@ export interface Info { environment?: Record extensions: string[] enabled(): Promise + buildRangeCommand?(file: string, ranges: DiffRange[]): string[] } export const gofmt: Info = { @@ -73,6 +75,25 @@ export const prettier: Info = { } return false }, + buildRangeCommand(file: string, ranges: DiffRange[]) { + // Prettier only supports a single range, so we merge all ranges into one + if (ranges.length === 0) { + return [BunProc.which(), "x", "prettier", "--write", file] + } + + // Merge all ranges into one for prettier + const merged = ranges.reduce((acc, range) => acc.merge(range), ranges[0]) + + return [ + BunProc.which(), + "x", + "prettier", + "--write", + `--range-start=${merged.start}`, + `--range-end=${merged.end}`, + file, + ] + }, } export const oxfmt: Info = { @@ -157,6 +178,21 @@ export const clang: Info = { const items = await Filesystem.findUp(".clang-format", Instance.directory, Instance.worktree) return items.length > 0 }, + buildRangeCommand(file: string, ranges: DiffRange[]) { + const cmd = ["clang-format", "-i"] + + // clang-format requires byte offsets - we must have cached values + for (const range of ranges) { + const byteOffsets = range.getCachedByteOffsets() + if (!byteOffsets) { + throw new Error("clang-format requires byte offsets but none were cached for range") + } + cmd.push(`--offset=${byteOffsets.start}`) + cmd.push(`--length=${byteOffsets.end - byteOffsets.start}`) + } + cmd.push(file) + return cmd + }, } export const ktlint: Info = { diff --git a/packages/opencode/src/format/index.ts b/packages/opencode/src/format/index.ts index bab758030b9..21d37583530 100644 --- a/packages/opencode/src/format/index.ts +++ b/packages/opencode/src/format/index.ts @@ -8,6 +8,7 @@ import * as Formatter from "./formatter" import { Config } from "../config/config" import { mergeDeep } from "remeda" import { Instance } from "../project/instance" +import { DiffRange } from "./diff-range" export namespace Format { const log = Log.create({ service: "format" }) @@ -104,14 +105,33 @@ export namespace Format { log.info("init") Bus.subscribe(File.Event.Edited, async (payload) => { const file = payload.properties.file - log.info("formatting", { file }) + const changedRanges = payload.properties.changedRanges + log.info("formatting", { file, changedRanges }) const ext = path.extname(file) for (const item of await getFormatter(ext)) { log.info("running", { command: item.command }) try { + let cmd: string[] + + // Use range formatting if supported and ranges are provided + if (item.buildRangeCommand && changedRanges) { + // Convert plain objects back to DiffRange instances + const rangeObjects = changedRanges.map((data) => DiffRange.fromJSON(data)) + + if (rangeObjects.length > 0) { + cmd = item.buildRangeCommand(file, rangeObjects) + log.info("using range formatting", { ranges: rangeObjects }) + } else { + log.info("formatting skipped: no changed ranges detected", { file }) + continue + } + } else { + cmd = item.command.map((x) => x.replace("$FILE", file)) + } + const proc = Bun.spawn({ - cmd: item.command.map((x) => x.replace("$FILE", file)), + cmd, cwd: Instance.directory, env: { ...process.env, ...item.environment }, stdout: "ignore", @@ -120,7 +140,7 @@ export namespace Format { const exit = await proc.exited if (exit !== 0) log.error("failed", { - command: item.command, + command: cmd, ...item.environment, }) } catch (error) { diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 0bf1d6792bc..ccfe076ef3a 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -17,6 +17,7 @@ import { Filesystem } from "../util/filesystem" import { Instance } from "../project/instance" import { Snapshot } from "@/snapshot" import { assertExternalDirectory } from "./external-directory" +import { calculateRanges } from "../format/diff-range" const MAX_DIAGNOSTICS_PER_FILE = 20 @@ -62,6 +63,8 @@ export const EditTool = Tool.define("edit", { }, }) await Bun.write(filePath, params.newString) + + // For new file content, format the entire file await Bus.publish(File.Event.Edited, { file: filePath, }) @@ -95,8 +98,12 @@ export const EditTool = Tool.define("edit", { }) await file.write(contentNew) + + const ranges = calculateRanges(contentOld, contentNew) + await Bus.publish(File.Event.Edited, { file: filePath, + changedRanges: ranges.map((r) => r.toJSON()), }) await Bus.publish(FileWatcher.Event.Updated, { file: filePath, diff --git a/packages/opencode/test/diff-range.test.ts b/packages/opencode/test/diff-range.test.ts new file mode 100644 index 00000000000..de6b47c99d5 --- /dev/null +++ b/packages/opencode/test/diff-range.test.ts @@ -0,0 +1,231 @@ +import { describe, expect, test } from "bun:test" +import { calculateRanges, DiffRange } from "../src/format/diff-range" + +describe("calculateRanges", () => { + test("calculates ranges for added lines", () => { + const oldContent = "line1\nline2\nline3" + const newContent = "line1\nline2\nnewline\nline3" + + const ranges = calculateRanges(oldContent, newContent) + + // "newline\n" starts at offset 12 (after "line1\nline2\n") and has length 8 + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(12) + expect(ranges[0]!.end).toBe(20) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 12, end: 20 }) + }) + + test("calculates ranges for multiple added lines", () => { + const oldContent = "line1\nline2\nline3" + const newContent = "line1\nline2\nnewline1\nnewline2\nnewline3\nline3" + + const ranges = calculateRanges(oldContent, newContent) + + // "newline1\nnewline2\nnewline3\n" starts at offset 12 and has length 27 + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(12) + expect(ranges[0]!.end).toBe(39) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 12, end: 39 }) + }) + + test("calculates ranges for removed lines", () => { + const oldContent = "line1\nline2\nline3\nline4" + const newContent = "line1\nline2\nline4" + + const ranges = calculateRanges(oldContent, newContent) + + // "line3\n" was removed at offset 12 (after "line1\nline2\n") + // Should report a zero-length range to trigger formatting at the join point + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(12) + expect(ranges[0]!.end).toBe(12) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 12, end: 12 }) + }) + + test("calculates ranges for removed lines at end", () => { + const oldContent = "line1\nline2\nline3" + const newContent = "line1\nline2" + + const ranges = calculateRanges(oldContent, newContent) + + // diffLines may report line2 as changed when line3 is removed (missing newline) + // In this case "line2" changed from "line2\n" to "line2", so offset 6, length 5 + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(6) + expect(ranges[0]!.end).toBe(11) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 6, end: 11 }) + }) + + test("merges adjacent ranges", () => { + const oldContent = "line1\nline2\nline3\nline4\nline5" + const newContent = "line1\nnew2\nline3\nnew4\nline5" + + const ranges = calculateRanges(oldContent, newContent) + + // "new2\n" at offset 6, length 5, and "new4\n" at offset 17, length 5 + // These should be merged since they're close + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(6) + expect(ranges[0]!.end).toBe(22) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 6, end: 22 }) + }) + + test("keeps separate ranges when not adjacent", () => { + const oldContent = "line1\nline2\nline3\nline4\nline5\nline6" + const newContent = "line1\nnew2\nline3\nline4\nline5\nnew6" + + const ranges = calculateRanges(oldContent, newContent) + + // "new2\n" at offset 6 and "new6" at offset 29 (counted properly) + expect(ranges.length).toBe(2) + expect(ranges[0]!.start).toBe(6) + expect(ranges[0]!.end).toBe(11) + expect(ranges[1]!.start).toBe(29) + expect(ranges[1]!.end).toBe(33) + }) + + test("handles empty old content", () => { + const oldContent = "" + const newContent = "line1\nline2\nline3" + + const ranges = calculateRanges(oldContent, newContent) + + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(0) + expect(ranges[0]!.end).toBe(17) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 0, end: 17 }) + }) + + test("handles complex edit with insertions and deletions", () => { + const oldContent = "line1\nline2\nline3\nline4\nline5" + const newContent = "line1\nnewA\nnewB\nline4\nline5" + + const ranges = calculateRanges(oldContent, newContent) + + // "newA\nnewB\n" starts at offset 6, length 10 + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(6) + expect(ranges[0]!.end).toBe(16) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 6, end: 16 }) + }) + + test("handles adding lines at the beginning", () => { + const oldContent = "line2\nline3" + const newContent = "line1\nline2\nline3" + + const ranges = calculateRanges(oldContent, newContent) + + // "line1\n" at offset 0, length 6 + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(0) + expect(ranges[0]!.end).toBe(6) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 0, end: 6 }) + }) + + test("handles adding lines at the end", () => { + const oldContent = "line1\nline2\n" + const newContent = "line1\nline2\nline3\n" + + const ranges = calculateRanges(oldContent, newContent) + + // "line3\n" starts at offset 12, length 6 + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(12) + expect(ranges[0]!.end).toBe(18) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 12, end: 18 }) + }) + + test("returns empty array for identical content", () => { + const content = "line1\nline2\nline3" + + const ranges = calculateRanges(content, content) + + expect(ranges).toEqual([]) + }) + + test("ignores line ending differences", () => { + const oldContent = "line1\r\nline2\r\nline3" + const newContent = "line1\nline2\nline3" + + const ranges = calculateRanges(oldContent, newContent) + + expect(ranges).toEqual([]) + }) + + test("handles byte-level accuracy for unicode", () => { + const oldContent = "hello\nworld" + const newContent = "hello\n世界" // "世界" is "world" in Chinese + + const ranges = calculateRanges(oldContent, newContent) + + // "世界" starts at offset 6 (after "hello\n") + // char length is 2 (2 JS chars) + // byte length is 6 (each Chinese char is 3 bytes in UTF-8) + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(6) + expect(ranges[0]!.end).toBe(8) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 6, end: 12 }) + }) + + test("handles unicode characters correctly in offsets", () => { + const oldContent = "a" + const newContent = "a\n💩" + + const ranges = calculateRanges(oldContent, newContent) + + // "a" (no newline) changed to "a\n..." is treated as a change of the first line + // So the range covers the entire new content + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(0) + expect(ranges[0]!.end).toBe(4) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 0, end: 6 }) + }) + + test("handles unicode with previous content containing unicode", () => { + const oldContent = "💩" + const newContent = "💩\nbar" + + const ranges = calculateRanges(oldContent, newContent) + + // Same here, "💩" changed to "💩\n..." + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(0) + expect(ranges[0]!.end).toBe(6) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 0, end: 8 }) + }) + + test("clamps offset to EOF when deleting last line", () => { + const oldContent = "line1\nline2" + const newContent = "line1\n" + + const ranges = calculateRanges(oldContent, newContent) + + // "line2" removed. newOffset would be 6 (length of "line1\n"). + // "line1\n" is 6 chars. valid offsets 0..5. + // Should clamp to 5. + expect(ranges.length).toBe(1) + expect(ranges[0]!.start).toBe(5) + expect(ranges[0]!.end).toBe(5) + expect(ranges[0]!.getCachedByteOffsets()).toEqual({ start: 5, end: 5 }) + }) +}) + +describe("DiffRange", () => { + test("shouldMerge works correctly", () => { + const range1 = new DiffRange(0, 5) + const range2 = new DiffRange(6, 10) // 1 char gap - should merge + const range3 = new DiffRange(15, 20) // 5 char gap - should not merge + + expect(range1.shouldMerge(range2)).toBe(true) + expect(range1.shouldMerge(range3)).toBe(false) + }) + + test("merge works correctly", () => { + const range1 = new DiffRange(0, 5) + const range2 = new DiffRange(6, 10) + + const merged = range1.merge(range2) + expect(merged.start).toBe(0) + expect(merged.end).toBe(10) + }) +})