Skip to content

Commit 6c236c2

Browse files
joaomorenojoaomoreno
committed
dirty diff should not include char changes
1 parent 5807d04 commit 6c236c2

3 files changed

Lines changed: 69 additions & 43 deletions

File tree

‎src/vs/editor/common/diff/diffComputer.ts‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ class LineChange implements ILineChange {
255255
this.charChanges = charChanges;
256256
}
257257

258-
public static createFromDiffResult(shouldIgnoreTrimWhitespace: boolean, diffChange: IDiffChange, originalLineSequence: LineMarkerSequence, modifiedLineSequence: LineMarkerSequence, continueProcessingPredicate: () => boolean, shouldPostProcessCharChanges: boolean): LineChange {
258+
public static createFromDiffResult(shouldIgnoreTrimWhitespace: boolean, diffChange: IDiffChange, originalLineSequence: LineMarkerSequence, modifiedLineSequence: LineMarkerSequence, continueProcessingPredicate: () => boolean, shouldComputeCharChanges: boolean, shouldPostProcessCharChanges: boolean): LineChange {
259259
let originalStartLineNumber: number;
260260
let originalEndLineNumber: number;
261261
let modifiedStartLineNumber: number;
@@ -278,7 +278,7 @@ class LineChange implements ILineChange {
278278
modifiedEndLineNumber = modifiedLineSequence.getEndLineNumber(diffChange.modifiedStart + diffChange.modifiedLength - 1);
279279
}
280280

281-
if (diffChange.originalLength !== 0 && diffChange.modifiedLength !== 0 && continueProcessingPredicate()) {
281+
if (shouldComputeCharChanges && diffChange.originalLength !== 0 && diffChange.modifiedLength !== 0 && continueProcessingPredicate()) {
282282
const originalCharSequence = originalLineSequence.getCharSequence(shouldIgnoreTrimWhitespace, diffChange.originalStart, diffChange.originalStart + diffChange.originalLength - 1);
283283
const modifiedCharSequence = modifiedLineSequence.getCharSequence(shouldIgnoreTrimWhitespace, diffChange.modifiedStart, diffChange.modifiedStart + diffChange.modifiedLength - 1);
284284

@@ -299,13 +299,15 @@ class LineChange implements ILineChange {
299299
}
300300

301301
export interface IDiffComputerOpts {
302+
shouldComputeCharChanges: boolean;
302303
shouldPostProcessCharChanges: boolean;
303304
shouldIgnoreTrimWhitespace: boolean;
304305
shouldMakePrettyDiff: boolean;
305306
}
306307

307308
export class DiffComputer {
308309

310+
private readonly shouldComputeCharChanges: boolean;
309311
private readonly shouldPostProcessCharChanges: boolean;
310312
private readonly shouldIgnoreTrimWhitespace: boolean;
311313
private readonly shouldMakePrettyDiff: boolean;
@@ -318,6 +320,7 @@ export class DiffComputer {
318320
private computationStartTime: number;
319321

320322
constructor(originalLines: string[], modifiedLines: string[], opts: IDiffComputerOpts) {
323+
this.shouldComputeCharChanges = opts.shouldComputeCharChanges;
321324
this.shouldPostProcessCharChanges = opts.shouldPostProcessCharChanges;
322325
this.shouldIgnoreTrimWhitespace = opts.shouldIgnoreTrimWhitespace;
323326
this.shouldMakePrettyDiff = opts.shouldMakePrettyDiff;
@@ -380,7 +383,7 @@ export class DiffComputer {
380383
if (this.shouldIgnoreTrimWhitespace) {
381384
let lineChanges: LineChange[] = [];
382385
for (let i = 0, length = rawChanges.length; i < length; i++) {
383-
lineChanges.push(LineChange.createFromDiffResult(this.shouldIgnoreTrimWhitespace, rawChanges[i], this.original, this.modified, this._continueProcessingPredicate.bind(this), this.shouldPostProcessCharChanges));
386+
lineChanges.push(LineChange.createFromDiffResult(this.shouldIgnoreTrimWhitespace, rawChanges[i], this.original, this.modified, this._continueProcessingPredicate.bind(this), this.shouldComputeCharChanges, this.shouldPostProcessCharChanges));
384387
}
385388
return lineChanges;
386389
}
@@ -455,7 +458,7 @@ export class DiffComputer {
455458

456459
if (nextChange) {
457460
// Emit the actual change
458-
result.push(LineChange.createFromDiffResult(this.shouldIgnoreTrimWhitespace, nextChange, this.original, this.modified, this._continueProcessingPredicate.bind(this), this.shouldPostProcessCharChanges));
461+
result.push(LineChange.createFromDiffResult(this.shouldIgnoreTrimWhitespace, nextChange, this.original, this.modified, this._continueProcessingPredicate.bind(this), this.shouldComputeCharChanges, this.shouldPostProcessCharChanges));
459462

460463
originalLineIndex += nextChange.originalLength;
461464
modifiedLineIndex += nextChange.modifiedLength;
@@ -475,15 +478,17 @@ export class DiffComputer {
475478
return;
476479
}
477480

481+
let charChanges: CharChange[];
482+
if (this.shouldComputeCharChanges) {
483+
charChanges = [new CharChange(
484+
originalLineNumber, originalStartColumn, originalLineNumber, originalEndColumn,
485+
modifiedLineNumber, modifiedStartColumn, modifiedLineNumber, modifiedEndColumn
486+
)];
487+
}
478488
result.push(new LineChange(
479489
originalLineNumber, originalLineNumber,
480490
modifiedLineNumber, modifiedLineNumber,
481-
[
482-
new CharChange(
483-
originalLineNumber, originalStartColumn, originalLineNumber, originalEndColumn,
484-
modifiedLineNumber, modifiedStartColumn, modifiedLineNumber, modifiedEndColumn
485-
)
486-
]
491+
charChanges
487492
));
488493
}
489494

@@ -507,10 +512,12 @@ export class DiffComputer {
507512
if (prevChange.originalEndLineNumber + 1 === originalLineNumber && prevChange.modifiedEndLineNumber + 1 === modifiedLineNumber) {
508513
prevChange.originalEndLineNumber = originalLineNumber;
509514
prevChange.modifiedEndLineNumber = modifiedLineNumber;
510-
prevChange.charChanges.push(new CharChange(
511-
originalLineNumber, originalStartColumn, originalLineNumber, originalEndColumn,
512-
modifiedLineNumber, modifiedStartColumn, modifiedLineNumber, modifiedEndColumn
513-
));
515+
if (this.shouldComputeCharChanges) {
516+
prevChange.charChanges.push(new CharChange(
517+
originalLineNumber, originalStartColumn, originalLineNumber, originalEndColumn,
518+
modifiedLineNumber, modifiedStartColumn, modifiedLineNumber, modifiedEndColumn
519+
));
520+
}
514521
return true;
515522
}
516523

‎src/vs/editor/common/services/editorSimpleWorker.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ export abstract class BaseEditorSimpleWorker {
332332
let originalLines = original.getLinesContent();
333333
let modifiedLines = modified.getLinesContent();
334334
let diffComputer = new DiffComputer(originalLines, modifiedLines, {
335+
shouldComputeCharChanges: true,
335336
shouldPostProcessCharChanges: true,
336337
shouldIgnoreTrimWhitespace: ignoreTrimWhitespace,
337338
shouldMakePrettyDiff: true
@@ -349,6 +350,7 @@ export abstract class BaseEditorSimpleWorker {
349350
let originalLines = original.getLinesContent();
350351
let modifiedLines = modified.getLinesContent();
351352
let diffComputer = new DiffComputer(originalLines, modifiedLines, {
353+
shouldComputeCharChanges: false,
352354
shouldPostProcessCharChanges: false,
353355
shouldIgnoreTrimWhitespace: ignoreTrimWhitespace,
354356
shouldMakePrettyDiff: true

‎src/vs/editor/test/common/diff/diffComputer.test.ts‎

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ function extractCharChangeRepresentation(change: ICharChange, expectedChange: IC
2525
}
2626

2727
function extractLineChangeRepresentation(change: ILineChange, expectedChange: ILineChange): IChange | ILineChange {
28+
let charChanges: ICharChange[];
2829
if (change.charChanges) {
29-
let charChanges: ICharChange[] = [];
30+
charChanges = [];
3031
for (let i = 0; i < change.charChanges.length; i++) {
3132
charChanges.push(
3233
extractCharChangeRepresentation(
@@ -35,26 +36,21 @@ function extractLineChangeRepresentation(change: ILineChange, expectedChange: IL
3536
)
3637
);
3738
}
38-
return {
39-
originalStartLineNumber: change.originalStartLineNumber,
40-
originalEndLineNumber: change.originalEndLineNumber,
41-
modifiedStartLineNumber: change.modifiedStartLineNumber,
42-
modifiedEndLineNumber: change.modifiedEndLineNumber,
43-
charChanges: charChanges
44-
};
4539
}
4640
return {
4741
originalStartLineNumber: change.originalStartLineNumber,
4842
originalEndLineNumber: change.originalEndLineNumber,
4943
modifiedStartLineNumber: change.modifiedStartLineNumber,
50-
modifiedEndLineNumber: change.modifiedEndLineNumber
44+
modifiedEndLineNumber: change.modifiedEndLineNumber,
45+
charChanges: charChanges
5146
};
5247
}
5348

54-
function assertDiff(originalLines: string[], modifiedLines: string[], expectedChanges: IChange[], shouldPostProcessCharChanges: boolean = false, shouldIgnoreTrimWhitespace: boolean = false) {
49+
function assertDiff(originalLines: string[], modifiedLines: string[], expectedChanges: IChange[], shouldComputeCharChanges: boolean = true, shouldPostProcessCharChanges: boolean = false, shouldIgnoreTrimWhitespace: boolean = false) {
5550
let diffComputer = new DiffComputer(originalLines, modifiedLines, {
56-
shouldPostProcessCharChanges: shouldPostProcessCharChanges || false,
57-
shouldIgnoreTrimWhitespace: shouldIgnoreTrimWhitespace || false,
51+
shouldComputeCharChanges,
52+
shouldPostProcessCharChanges,
53+
shouldIgnoreTrimWhitespace,
5854
shouldMakePrettyDiff: true
5955
});
6056
let changes = diffComputer.computeDiff();
@@ -66,25 +62,27 @@ function assertDiff(originalLines: string[], modifiedLines: string[], expectedCh
6662
assert.deepEqual(extracted, expectedChanges);
6763
}
6864

69-
function createLineDeletion(startLineNumber: number, endLineNumber: number, modifiedLineNumber: number): IChange {
65+
function createLineDeletion(startLineNumber: number, endLineNumber: number, modifiedLineNumber: number): ILineChange {
7066
return {
7167
originalStartLineNumber: startLineNumber,
7268
originalEndLineNumber: endLineNumber,
7369
modifiedStartLineNumber: modifiedLineNumber,
74-
modifiedEndLineNumber: 0
70+
modifiedEndLineNumber: 0,
71+
charChanges: undefined
7572
};
7673
}
7774

78-
function createLineInsertion(startLineNumber: number, endLineNumber: number, originalLineNumber: number): IChange {
75+
function createLineInsertion(startLineNumber: number, endLineNumber: number, originalLineNumber: number): ILineChange {
7976
return {
8077
originalStartLineNumber: originalLineNumber,
8178
originalEndLineNumber: 0,
8279
modifiedStartLineNumber: startLineNumber,
83-
modifiedEndLineNumber: endLineNumber
80+
modifiedEndLineNumber: endLineNumber,
81+
charChanges: undefined
8482
};
8583
}
8684

87-
function createLineChange(originalStartLineNumber: number, originalEndLineNumber: number, modifiedStartLineNumber: number, modifiedEndLineNumber: number, charChanges: ICharChange[]): ILineChange {
85+
function createLineChange(originalStartLineNumber: number, originalEndLineNumber: number, modifiedStartLineNumber: number, modifiedEndLineNumber: number, charChanges?: ICharChange[]): ILineChange {
8886
return {
8987
originalStartLineNumber: originalStartLineNumber,
9088
originalEndLineNumber: originalEndLineNumber,
@@ -390,7 +388,7 @@ suite('Editor Diff - DiffComputer', () => {
390388
createCharChange(1, 2, 1, 4, 1, 2, 1, 13)
391389
])
392390
];
393-
assertDiff(original, modified, expected, true);
391+
assertDiff(original, modified, expected, true, true);
394392
});
395393

396394
test('ignore trim whitespace', () => {
@@ -403,7 +401,7 @@ suite('Editor Diff - DiffComputer', () => {
403401
createCharInsertion(4, 1, 4, 4)
404402
])
405403
];
406-
assertDiff(original, modified, expected, false, true);
404+
assertDiff(original, modified, expected, true, false, true);
407405
});
408406

409407
test('issue #12122 r.hasOwnProperty is not a function', () => {
@@ -423,7 +421,7 @@ suite('Editor Diff - DiffComputer', () => {
423421
createCharChange(0, 0, 0, 0, 0, 0, 0, 0)
424422
])
425423
];
426-
assertDiff(original, modified, expected, false, true);
424+
assertDiff(original, modified, expected, true, false, true);
427425
});
428426

429427
test('empty diff 2', () => {
@@ -434,7 +432,7 @@ suite('Editor Diff - DiffComputer', () => {
434432
createCharChange(0, 0, 0, 0, 0, 0, 0, 0)
435433
])
436434
];
437-
assertDiff(original, modified, expected, false, true);
435+
assertDiff(original, modified, expected, true, false, true);
438436
});
439437

440438
test('empty diff 3', () => {
@@ -445,7 +443,7 @@ suite('Editor Diff - DiffComputer', () => {
445443
createCharChange(0, 0, 0, 0, 0, 0, 0, 0)
446444
])
447445
];
448-
assertDiff(original, modified, expected, false, true);
446+
assertDiff(original, modified, expected, true, false, true);
449447
});
450448

451449
test('empty diff 4', () => {
@@ -456,7 +454,7 @@ suite('Editor Diff - DiffComputer', () => {
456454
createCharChange(0, 0, 0, 0, 0, 0, 0, 0)
457455
])
458456
];
459-
assertDiff(original, modified, expected, false, true);
457+
assertDiff(original, modified, expected, true, false, true);
460458
});
461459

462460
test('pretty diff 1', () => {
@@ -493,7 +491,7 @@ suite('Editor Diff - DiffComputer', () => {
493491
createLineInsertion(1, 1, 0),
494492
createLineInsertion(10, 13, 8)
495493
];
496-
assertDiff(original, modified, expected, false, true);
494+
assertDiff(original, modified, expected, true, false, true);
497495
});
498496

499497
test('pretty diff 2', () => {
@@ -536,7 +534,7 @@ suite('Editor Diff - DiffComputer', () => {
536534
createLineInsertion(1, 3, 0),
537535
createLineDeletion(10, 13, 12),
538536
];
539-
assertDiff(original, modified, expected, false, true);
537+
assertDiff(original, modified, expected, true, false, true);
540538
});
541539

542540
test('pretty diff 3', () => {
@@ -574,7 +572,7 @@ suite('Editor Diff - DiffComputer', () => {
574572
let expected = [
575573
createLineInsertion(7, 11, 6)
576574
];
577-
assertDiff(original, modified, expected, false, true);
575+
assertDiff(original, modified, expected, true, false, true);
578576
});
579577

580578
test('issue #23636', () => {
@@ -671,7 +669,7 @@ suite('Editor Diff - DiffComputer', () => {
671669
)
672670
// createLineInsertion(7, 11, 6)
673671
];
674-
assertDiff(original, modified, expected, true, false);
672+
assertDiff(original, modified, expected, true, true, false);
675673
});
676674

677675
test('issue #43922', () => {
@@ -690,7 +688,7 @@ suite('Editor Diff - DiffComputer', () => {
690688
]
691689
)
692690
];
693-
assertDiff(original, modified, expected, true, false);
691+
assertDiff(original, modified, expected, true, true, false);
694692
});
695693

696694
test('issue #42751', () => {
@@ -710,6 +708,25 @@ suite('Editor Diff - DiffComputer', () => {
710708
]
711709
)
712710
];
713-
assertDiff(original, modified, expected, true, false);
711+
assertDiff(original, modified, expected, true, true, false);
712+
});
713+
714+
test('does not give character changes', () => {
715+
let original = [
716+
' 1',
717+
' 2',
718+
'A',
719+
];
720+
let modified = [
721+
' 1',
722+
' 3',
723+
' A',
724+
];
725+
let expected = [
726+
createLineChange(
727+
2, 3, 2, 3
728+
)
729+
];
730+
assertDiff(original, modified, expected, false, false, false);
714731
});
715732
});

0 commit comments

Comments
 (0)