Skip to content

Commit

Permalink
Merge pull request microsoft#178142 from microsoft/hediet/diffing-wor…
Browse files Browse the repository at this point in the history
…d-expansion

Improves experimental diffing by selecting entire words if they are changed too much
  • Loading branch information
jrieken authored Mar 23, 2023
2 parents 0bdaa6f + 5684cdf commit 840a5e7
Show file tree
Hide file tree
Showing 15 changed files with 724 additions and 33 deletions.
21 changes: 21 additions & 0 deletions src/vs/editor/browser/services/editorWorkerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,21 @@ export class EditorWorkerService extends Disposable implements IEditorWorkerServ
}
}

public computeHumanReadableDiff(resource: URI, edits: languages.TextEdit[] | null | undefined): Promise<languages.TextEdit[] | undefined> {
if (isNonEmptyArray(edits)) {
if (!canSyncModel(this._modelService, resource)) {
return Promise.resolve(edits); // File too large
}
const sw = StopWatch.create(true);
const result = this._workerManager.withWorker().then(client => client.computeHumanReadableDiff(resource, edits));
result.finally(() => this._logService.trace('FORMAT#computeHumanReadableDiff', resource.toString(true), sw.elapsed()));
return Promise.race([result, timeout(1000).then(() => edits)]);

} else {
return Promise.resolve(undefined);
}
}

public canNavigateValueSet(resource: URI): boolean {
return (canSyncModel(this._modelService, resource));
}
Expand Down Expand Up @@ -535,6 +550,12 @@ export class EditorWorkerClient extends Disposable implements IEditorWorkerClien
});
}

public computeHumanReadableDiff(resource: URI, edits: languages.TextEdit[]): Promise<languages.TextEdit[]> {
return this._withSyncedResources([resource]).then(proxy => {
return proxy.computeHumanReadableDiff(resource.toString(), edits);
});
}

public computeLinks(resource: URI): Promise<languages.ILink[] | null> {
return this._withSyncedResources([resource]).then(proxy => {
return proxy.computeLinks(resource.toString());
Expand Down
17 changes: 17 additions & 0 deletions src/vs/editor/common/diff/algorithms/diffAlgorithm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ export class SequenceDiff {
public toString(): string {
return `${this.seq1Range} <-> ${this.seq2Range}`;
}

public join(other: SequenceDiff): SequenceDiff {
return new SequenceDiff(this.seq1Range.join(other.seq1Range), this.seq2Range.join(other.seq2Range));
}
}

/**
Expand Down Expand Up @@ -50,6 +54,19 @@ export class OffsetRange {
public join(other: OffsetRange): OffsetRange {
return new OffsetRange(Math.min(this.start, other.start), Math.max(this.endExclusive, other.endExclusive));
}

public equals(other: OffsetRange): boolean {
return this.start === other.start && this.endExclusive === other.endExclusive;
}

intersect(seq1Range: OffsetRange): OffsetRange | undefined {
const start = Math.max(this.start, seq1Range.start);
const end = Math.min(this.endExclusive, seq1Range.endExclusive);
if (start <= end) {
return new OffsetRange(start, end);
}
return undefined;
}
}

export interface ISequence {
Expand Down
129 changes: 128 additions & 1 deletion src/vs/editor/common/diff/standardLinesDiffComputer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,97 @@ export class StandardLinesDiffComputer implements ILinesDiffComputer {
? this.dynamicProgrammingDiffing.compute(sourceSlice, targetSlice)
: this.myersDiffingAlgorithm.compute(sourceSlice, targetSlice);

let diffs = optimizeSequenceDiffs(sourceSlice, targetSlice, originalDiffs);
function mergeSequenceDiffs(sequenceDiffs1: SequenceDiff[], sequenceDiffs2: SequenceDiff[]): SequenceDiff[] {
const result: SequenceDiff[] = [];

while (sequenceDiffs1.length > 0 || sequenceDiffs2.length > 0) {
const sd1 = sequenceDiffs1[0];
const sd2 = sequenceDiffs2[0];

let next: SequenceDiff;
if (sd1 && (!sd2 || sd1.seq1Range.start < sd2.seq1Range.start)) {
next = sequenceDiffs1.shift()!;
} else {
next = sequenceDiffs2.shift()!;
}

if (result.length > 0 && result[result.length - 1].seq1Range.endExclusive >= next.seq1Range.start) {
result[result.length - 1] = result[result.length - 1].join(next);
} else {
result.push(next);
}
}

return result;
}

function coverFullWords(sequence1: Slice, sequence2: Slice, sequenceDiffs: SequenceDiff[]): SequenceDiff[] {
const additional: SequenceDiff[] = [];

let lastModifiedWord: { added: number; deleted: number; count: number; s1Range: OffsetRange; s2Range: OffsetRange } | undefined = undefined;

function maybePushWordToAdditional() {
if (!lastModifiedWord) {
return;
}

const originalLength1 = lastModifiedWord.s1Range.length - lastModifiedWord.deleted;
const originalLength2 = lastModifiedWord.s2Range.length - lastModifiedWord.added;
if (originalLength1 !== originalLength2) {
lastModifiedWord = undefined;
return; // TODO figure out why this happens
}

if (Math.max(lastModifiedWord.deleted, lastModifiedWord.added) + (lastModifiedWord.count - 1) > originalLength1) {
additional.push(new SequenceDiff(lastModifiedWord.s1Range, lastModifiedWord.s2Range));
}

lastModifiedWord = undefined;
}

for (const s of sequenceDiffs) {
function processWord(s1Range: OffsetRange, s2Range: OffsetRange) {
if (!lastModifiedWord || !lastModifiedWord.s1Range.equals(s1Range) || !lastModifiedWord.s2Range.equals(s2Range)) {
if (lastModifiedWord && lastModifiedWord.s1Range.endExclusive < s1Range.start && lastModifiedWord.s2Range.endExclusive < s2Range.start) {
maybePushWordToAdditional();
}
lastModifiedWord = { added: 0, deleted: 0, count: 0, s1Range: s1Range, s2Range: s2Range };
}

const changedS1 = s1Range.intersect(s.seq1Range);
const changedS2 = s2Range.intersect(s.seq2Range);
lastModifiedWord.count++;
lastModifiedWord.added += changedS2?.length ?? 0;
lastModifiedWord.deleted += changedS1?.length ?? 0;
}

const w1Before = sequence1.findWordContaining(s.seq1Range.start - 1);
const w2Before = sequence2.findWordContaining(s.seq2Range.start - 1);

const w1After = sequence1.findWordContaining(s.seq1Range.endExclusive);
const w2After = sequence2.findWordContaining(s.seq2Range.endExclusive);

if (w1Before && w1After && w2Before && w2After && w1Before.equals(w1After) && w2Before.equals(w2After)) {
processWord(w1Before, w2Before);
} else {
if (w1Before && w2Before) {
processWord(w1Before, w2Before);
}
if (w1After && w2After) {
processWord(w1After, w2After);
}
}
}

maybePushWordToAdditional();

const merged = mergeSequenceDiffs(sequenceDiffs, additional);
return merged;
}

let diffs = originalDiffs;
diffs = coverFullWords(sourceSlice, targetSlice, diffs);
diffs = optimizeSequenceDiffs(sourceSlice, targetSlice, diffs);
diffs = smoothenSequenceDiffs(sourceSlice, targetSlice, diffs);
const result = diffs.map(
(d) =>
Expand Down Expand Up @@ -246,6 +336,10 @@ class Slice implements ISequence {
}
}

toString() {
return `Slice: "${this.text}"`;
}

get text(): string {
return [...this.elements].map(e => String.fromCharCode(e)).join('');
}
Expand Down Expand Up @@ -305,6 +399,39 @@ class Slice implements ISequence {
public translateRange(range: OffsetRange): Range {
return Range.fromPositions(this.translateOffset(range.start), this.translateOffset(range.endExclusive));
}

/**
* Finds the word that contains the character at the given offset
*/
public findWordContaining(offset: number): OffsetRange | undefined {
if (offset < 0 || offset >= this.elements.length) {
return undefined;
}

if (!isWordChar(this.elements[offset])) {
return undefined;
}

// find start
let start = offset;
while (start > 0 && isWordChar(this.elements[start - 1])) {
start--;
}

// find end
let end = offset;
while (end < this.elements.length && isWordChar(this.elements[end])) {
end++;
}

return new OffsetRange(start, end);
}
}

function isWordChar(charCode: number): boolean {
return charCode >= CharCode.a && charCode <= CharCode.z
|| charCode >= CharCode.A && charCode <= CharCode.Z
|| charCode >= CharCode.Digit0 && charCode <= CharCode.Digit9;
}

const enum CharBoundaryCategory {
Expand Down
128 changes: 127 additions & 1 deletion src/vs/editor/common/services/editorSimpleWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ import { IEditorWorkerHost } from 'vs/editor/common/services/editorWorkerHost';
import { StopWatch } from 'vs/base/common/stopwatch';
import { UnicodeTextModelHighlighter, UnicodeHighlighterOptions } from 'vs/editor/common/services/unicodeTextModelHighlighter';
import { DiffComputer, IChange } from 'vs/editor/common/diff/smartLinesDiffComputer';
import { ILinesDiffComputer } from 'vs/editor/common/diff/linesDiffComputer';
import { ILinesDiffComputer, LineRangeMapping, RangeMapping } from 'vs/editor/common/diff/linesDiffComputer';
import { linesDiffComputers } from 'vs/editor/common/diff/linesDiffComputers';
import { createProxyObject, getAllMethodNames } from 'vs/base/common/objects';
import { IDocumentDiffProviderOptions } from 'vs/editor/common/diff/documentDiffProvider';
import { BugIndicatingError } from 'vs/base/common/errors';

export interface IMirrorModel extends IMirrorTextModel {
readonly uri: URI;
Expand Down Expand Up @@ -530,6 +531,131 @@ export class EditorSimpleWorker implements IRequestHandler, IDisposable {
return result;
}

public async computeHumanReadableDiff(modelUrl: string, edits: TextEdit[]): Promise<TextEdit[]> {
const model = this._getModel(modelUrl);
if (!model) {
return edits;
}

const result: TextEdit[] = [];
let lastEol: EndOfLineSequence | undefined = undefined;

edits = edits.slice(0).sort((a, b) => {
if (a.range && b.range) {
return Range.compareRangesUsingStarts(a.range, b.range);
}
// eol only changes should go to the end
const aRng = a.range ? 0 : 1;
const bRng = b.range ? 0 : 1;
return aRng - bRng;
});

for (let { range, text, eol } of edits) {

if (typeof eol === 'number') {
lastEol = eol;
}

if (Range.isEmpty(range) && !text) {
// empty change
continue;
}

const original = model.getValueInRange(range);
text = text.replace(/\r\n|\n|\r/g, model.eol);

if (original === text) {
// noop
continue;
}

// make sure diff won't take too long
if (Math.max(text.length, original.length) > EditorSimpleWorker._diffLimit) {
result.push({ range, text });
continue;
}

// compute diff between original and edit.text

const originalLines = original.split(/\r\n|\n|\r/);
const modifiedLines = text.split(/\r\n|\n|\r/);

const diff = linesDiffComputers.experimental.computeDiff(originalLines, modifiedLines, { maxComputationTimeMs: 1000, ignoreTrimWhitespace: false });

// TODO this should be fixed in the experimental diff algorithm, but it might have consequences for the merge editor.
function moveUpInvalidInnerChanges(alignments: LineRangeMapping[], originalLineCount: number, modifiedLineCount: number): LineRangeMapping[] {
return alignments.map(a => {
if (!a.innerChanges) {
return a;
}
return new LineRangeMapping(a.originalRange, a.modifiedRange, a.innerChanges.map(c => {
if (c.originalRange.endColumn === 1 && c.originalRange.endLineNumber > originalLineCount) {
if (c.originalRange.isEmpty() || c.modifiedRange.isEmpty()) {
return new RangeMapping(
new Range(c.originalRange.startLineNumber - 1, Number.MAX_SAFE_INTEGER, c.originalRange.endLineNumber - 1, Number.MAX_SAFE_INTEGER),
new Range(c.modifiedRange.startLineNumber - 1, Number.MAX_SAFE_INTEGER, c.modifiedRange.endLineNumber - 1, Number.MAX_SAFE_INTEGER),
);
} else {
return new RangeMapping(
new Range(c.originalRange.startLineNumber, c.originalRange.startColumn, c.originalRange.endLineNumber - 1, Number.MAX_SAFE_INTEGER),
new Range(c.modifiedRange.startLineNumber, c.modifiedRange.startColumn, c.modifiedRange.endLineNumber - 1, Number.MAX_SAFE_INTEGER),
);
}
}
return c;
}));
});
}

const repairedAlignments = moveUpInvalidInnerChanges(diff.changes, originalLines.length, modifiedLines.length);

const start = Range.lift(range).getStartPosition();

function addPositions(pos1: Position, pos2: Position): Position {
return new Position(pos1.lineNumber + pos2.lineNumber - 1, pos2.lineNumber === 1 ? pos1.column + pos2.column - 1 : pos2.column);
}

function getText(lines: string[], range: Range): string[] {
const result: string[] = [];
for (let i = range.startLineNumber; i <= range.endLineNumber; i++) {
const line = lines[i - 1];
if (i === range.startLineNumber && i === range.endLineNumber) {
result.push(line.substring(range.startColumn - 1, range.endColumn - 1));
} else if (i === range.startLineNumber) {
result.push(line.substring(range.startColumn - 1));
} else if (i === range.endLineNumber) {
result.push(line.substring(0, range.endColumn - 1));
} else {
result.push(line);
}
}
return result;
}

for (const c of repairedAlignments) {
if (c.innerChanges) {
for (const x of c.innerChanges) {
result.push({
range: Range.fromPositions(
addPositions(start, x.originalRange.getStartPosition()),
addPositions(start, x.originalRange.getEndPosition())
),
text: getText(modifiedLines, x.modifiedRange).join(model.eol)
});
}
} else {
throw new BugIndicatingError('The experimental diff algorithm always produces inner changes');
}
}
}

if (typeof lastEol === 'number') {
result.push({ eol: lastEol, text: '', range: { startLineNumber: 0, startColumn: 0, endLineNumber: 0, endColumn: 0 } });
}

return result;
}

// ---- END minimal edits ---------------------------------------------------------------

public async computeLinks(modelUrl: string): Promise<ILink[] | null> {
Expand Down
1 change: 1 addition & 0 deletions src/vs/editor/common/services/editorWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface IEditorWorkerService {
computeDirtyDiff(original: URI, modified: URI, ignoreTrimWhitespace: boolean): Promise<IChange[] | null>;

computeMoreMinimalEdits(resource: URI, edits: TextEdit[] | null | undefined, pretty?: boolean): Promise<TextEdit[] | undefined>;
computeHumanReadableDiff(resource: URI, edits: TextEdit[] | null | undefined): Promise<TextEdit[] | undefined>;

canComputeWordRanges(resource: URI): boolean;
computeWordRanges(resource: URI, range: IRange): Promise<{ [word: string]: IRange[] } | null>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export class TestEditorWorkerService implements IEditorWorkerService {
canComputeDirtyDiff(original: URI, modified: URI): boolean { return false; }
async computeDirtyDiff(original: URI, modified: URI, ignoreTrimWhitespace: boolean): Promise<IChange[] | null> { return null; }
async computeMoreMinimalEdits(resource: URI, edits: TextEdit[] | null | undefined): Promise<TextEdit[] | undefined> { return undefined; }
async computeHumanReadableDiff(resource: URI, edits: TextEdit[] | null | undefined): Promise<TextEdit[] | undefined> { return undefined; }
canComputeWordRanges(resource: URI): boolean { return false; }
async computeWordRanges(resource: URI, range: IRange): Promise<{ [word: string]: IRange[] } | null> { return null; }
canNavigateValueSet(resource: URI): boolean { return false; }
Expand Down
Loading

0 comments on commit 840a5e7

Please sign in to comment.