Skip to content

Commit 1a80013

Browse files
authored
Use consistent diff algorithm for model and view (microsoft#239231)
use same diff algorithm across model and view
1 parent 45212e1 commit 1a80013

File tree

3 files changed

+61
-90
lines changed

3 files changed

+61
-90
lines changed

src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsModel.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -368,10 +368,6 @@ export class InlineCompletionsModel extends Disposable {
368368
const item = this._inlineCompletionItems.read(reader);
369369
const inlineEditResult = item?.inlineEdit;
370370
if (inlineEditResult) {
371-
if (inlineEditResult.inlineEdit.read(reader) === null) {
372-
return undefined;
373-
}
374-
375371
let edit = inlineEditResult.toSingleTextEdit(reader);
376372
edit = singleTextRemoveCommonPrefix(edit, model);
377373

@@ -767,14 +763,15 @@ export class InlineCompletionsModel extends Disposable {
767763
transaction(tx => {
768764
this._jumpedTo.set(true, tx);
769765
this.dontRefetchSignal.trigger(tx);
770-
this._editor.setPosition(s.inlineEdit.range.getStartPosition(), 'inlineCompletions.jump');
766+
const edit = s.inlineCompletion.toSingleTextEdit(undefined);
767+
this._editor.setPosition(edit.range.getStartPosition(), 'inlineCompletions.jump');
771768

772769
// TODO: consider using view information to reveal it
773-
const isSingleLineChange = s.inlineEdit.range.startLineNumber === s.inlineEdit.range.endLineNumber && !s.inlineEdit.text.includes('\n');
770+
const isSingleLineChange = edit.range.startLineNumber === edit.range.endLineNumber && !edit.text.includes('\n');
774771
if (isSingleLineChange) {
775-
this._editor.revealPosition(s.inlineEdit.range.getStartPosition());
772+
this._editor.revealPosition(edit.range.getStartPosition());
776773
} else {
777-
const revealRange = new Range(s.inlineEdit.range.startLineNumber - 1, 1, s.inlineEdit.range.endLineNumber + 1, 1);
774+
const revealRange = new Range(edit.range.startLineNumber - 1, 1, edit.range.endLineNumber + 1, 1);
778775
this._editor.revealRange(revealRange, ScrollType.Immediate);
779776
}
780777

src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsSource.ts

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { equalsIfDefined, itemEquals } from '../../../../../base/common/equals.j
88
import { matchesSubString } from '../../../../../base/common/filters.js';
99
import { Disposable, IDisposable, MutableDisposable } from '../../../../../base/common/lifecycle.js';
1010
import { IObservable, IReader, ISettableObservable, ITransaction, derivedOpts, disposableObservableValue, observableFromEvent, observableValue, transaction } from '../../../../../base/common/observable.js';
11+
import { splitLines } from '../../../../../base/common/strings.js';
1112
import { ICommandService } from '../../../../../platform/commands/common/commands.js';
1213
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
1314
import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js';
@@ -18,15 +19,15 @@ import { OffsetEdit, SingleOffsetEdit } from '../../../../common/core/offsetEdit
1819
import { OffsetRange } from '../../../../common/core/offsetRange.js';
1920
import { Position } from '../../../../common/core/position.js';
2021
import { Range } from '../../../../common/core/range.js';
21-
import { SingleTextEdit } from '../../../../common/core/textEdit.js';
22+
import { SingleTextEdit, StringText } from '../../../../common/core/textEdit.js';
2223
import { TextLength } from '../../../../common/core/textLength.js';
24+
import { linesDiffComputers } from '../../../../common/diff/linesDiffComputers.js';
2325
import { InlineCompletionContext, InlineCompletionTriggerKind } from '../../../../common/languages.js';
2426
import { ILanguageConfigurationService } from '../../../../common/languages/languageConfigurationRegistry.js';
2527
import { EndOfLinePreference, ITextModel } from '../../../../common/model.js';
2628
import { IFeatureDebounceInformation } from '../../../../common/services/languageFeatureDebounce.js';
2729
import { ILanguageFeaturesService } from '../../../../common/services/languageFeatures.js';
2830
import { IModelContentChange, IModelContentChangedEvent } from '../../../../common/textModelEvents.js';
29-
import { smartDiff } from './computeGhostText.js';
3031
import { InlineCompletionItem, InlineCompletionProviderResult, provideInlineCompletions } from './provideInlineCompletions.js';
3132
import { singleTextRemoveCommonPrefix } from './singleTextEditHelpers.js';
3233

@@ -376,21 +377,44 @@ export class InlineCompletionWithUpdatedRange {
376377
}
377378
}
378379

379-
private _toIndividualEdits(range: Range, _replaceText: string): OffsetEdit {
380-
const originalText = this._textModel.getValueInRange(range);
381-
const replaceText = _replaceText.replace(/\r\n|\r|\n/g, this._textModel.getEOL());
382-
const diffs = smartDiff(originalText, replaceText, false);
383-
const startOffset = this._textModel.getOffsetAt(range.getStartPosition());
384-
if (!diffs || diffs.length === 0) {
380+
private _toIndividualEdits(editRange: Range, _replaceText: string): OffsetEdit {
381+
const editOriginalText = this._textModel.getValueInRange(editRange);
382+
const editReplaceText = _replaceText.replace(/\r\n|\r|\n/g, this._textModel.getEOL());
383+
384+
const diffAlgorithm = linesDiffComputers.getDefault();
385+
const lineDiffs = diffAlgorithm.computeDiff(
386+
splitLines(editOriginalText),
387+
splitLines(editReplaceText),
388+
{
389+
ignoreTrimWhitespace: false,
390+
computeMoves: false,
391+
extendToSubwords: true,
392+
maxComputationTimeMs: 500,
393+
}
394+
);
395+
396+
const innerChanges = lineDiffs.changes.flatMap(c => c.innerChanges ?? []);
397+
if (innerChanges.length === 0) {
398+
const startOffset = this._textModel.getOffsetAt(editRange.getStartPosition());
385399
return new OffsetEdit(
386-
[new SingleOffsetEdit(OffsetRange.ofStartAndLength(startOffset, originalText.length), replaceText)]
400+
[new SingleOffsetEdit(OffsetRange.ofStartAndLength(startOffset, editOriginalText.length), editReplaceText)]
387401
);
388402
}
403+
404+
function addRangeToPos(pos: Position, range: Range): Range {
405+
const start = TextLength.fromPosition(range.getStartPosition());
406+
return TextLength.ofRange(range).createRange(start.addToPosition(pos));
407+
}
408+
409+
const modifiedText = new StringText(editReplaceText);
410+
389411
return new OffsetEdit(
390-
diffs.map(diff => {
391-
const originalRange = OffsetRange.ofStartAndLength(startOffset + diff.originalStart, diff.originalLength);
392-
const modifiedText = replaceText.substring(diff.modifiedStart, diff.modifiedStart + diff.modifiedLength);
393-
return new SingleOffsetEdit(originalRange, modifiedText);
412+
innerChanges.map(c => {
413+
const range = addRangeToPos(editRange.getStartPosition(), c.originalRange);
414+
const startOffset = this._textModel.getOffsetAt(range.getStartPosition());
415+
const endOffset = this._textModel.getOffsetAt(range.getEndPosition());
416+
const originalRange = OffsetRange.ofStartAndLength(startOffset, endOffset - startOffset);
417+
return new SingleOffsetEdit(originalRange, modifiedText.getValueOfRange(c.modifiedRange));
394418
})
395419
);
396420
}

src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/viewAndDiffProducer.ts

Lines changed: 19 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -3,106 +3,56 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { LRUCachedFunction } from '../../../../../../base/common/cache.js';
7-
import { CancellationToken } from '../../../../../../base/common/cancellation.js';
8-
import { equalsIfDefined, itemEquals } from '../../../../../../base/common/equals.js';
96
import { createHotClass } from '../../../../../../base/common/hotReloadHelpers.js';
107
import { Disposable } from '../../../../../../base/common/lifecycle.js';
11-
import { derivedDisposable, ObservablePromise, derived, IObservable, derivedOpts, ISettableObservable } from '../../../../../../base/common/observable.js';
8+
import { derived, IObservable, ISettableObservable } from '../../../../../../base/common/observable.js';
129
import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js';
1310
import { ICodeEditor } from '../../../../../browser/editorBrowser.js';
14-
import { IDiffProviderFactoryService } from '../../../../../browser/widget/diffEditor/diffProviderFactoryService.js';
1511
import { SingleLineEdit } from '../../../../../common/core/lineEdit.js';
1612
import { Position } from '../../../../../common/core/position.js';
1713
import { Range } from '../../../../../common/core/range.js';
1814
import { SingleTextEdit, TextEdit, AbstractText } from '../../../../../common/core/textEdit.js';
19-
import { TextLength } from '../../../../../common/core/textLength.js';
2015
import { Command } from '../../../../../common/languages.js';
2116
import { TextModelText } from '../../../../../common/model/textModelText.js';
22-
import { IModelService } from '../../../../../common/services/model.js';
2317
import { InlineCompletionsModel } from '../../model/inlineCompletionsModel.js';
2418
import { InlineEdit } from '../../model/inlineEdit.js';
2519
import { InlineCompletionItem } from '../../model/provideInlineCompletions.js';
2620
import { InlineEditsView } from './view.js';
27-
import { UniqueUriGenerator } from './utils.js';
2821

29-
export class InlineEditsViewAndDiffProducer extends Disposable {
22+
export class InlineEditsViewAndDiffProducer extends Disposable { // TODO: This class is no longer a diff producer. Rename it or get rid of it
3023
public static readonly hot = createHotClass(InlineEditsViewAndDiffProducer);
3124

32-
private readonly _modelUriGenerator = new UniqueUriGenerator('inline-edits');
33-
34-
private readonly _originalModel = derivedDisposable(() => this._modelService.createModel(
35-
'', null, this._modelUriGenerator.getUniqueUri())).keepObserved(this._store);
36-
private readonly _modifiedModel = derivedDisposable(() => this._modelService.createModel(
37-
'', null, this._modelUriGenerator.getUniqueUri())).keepObserved(this._store);
38-
39-
private readonly _differ = new LRUCachedFunction({ getCacheKey: JSON.stringify }, (arg: { original: string; modified: string }) => {
40-
this._originalModel.get().setValue(arg.original);
41-
this._modifiedModel.get().setValue(arg.modified);
42-
43-
const diffAlgo = this._diffProviderFactoryService.createDiffProvider({ diffAlgorithm: 'advanced' });
44-
45-
return ObservablePromise.fromFn(async () => {
46-
const result = await diffAlgo.computeDiff(this._originalModel.get(), this._modifiedModel.get(), {
47-
computeMoves: false,
48-
ignoreTrimWhitespace: false,
49-
maxComputationTimeMs: 1000,
50-
extendToSubwords: true,
51-
}, CancellationToken.None);
52-
return result;
53-
});
54-
});
55-
56-
private readonly _inlineEditPromise = derived<IObservable<InlineEditWithChanges | undefined> | undefined>(this, (reader) => {
25+
private readonly _inlineEdit = derived<InlineEditWithChanges | undefined>(this, (reader) => {
5726
const model = this._model.read(reader);
5827
if (!model) { return undefined; }
5928
const inlineEdit = this._edit.read(reader);
6029
if (!inlineEdit) { return undefined; }
30+
const textModel = this._editor.getModel();
31+
if (!textModel) { return undefined; }
32+
33+
const editOffset = model.inlineEditState.get()?.inlineCompletion.inlineEdit.read(reader);
34+
if (!editOffset) { return undefined; }
35+
36+
const edits = editOffset.edits.map(e => {
37+
const innerEditRange = Range.fromPositions(
38+
textModel.getPositionAt(e.replaceRange.start),
39+
textModel.getPositionAt(e.replaceRange.endExclusive)
40+
);
41+
return new SingleTextEdit(innerEditRange, e.newText);
42+
});
6143

62-
//if (inlineEdit.text.trim() === '') { return undefined; }
63-
const text = new TextModelText(this._editor.getModel()!);
64-
const edit = inlineEdit.edit.extendToFullLine(text);
65-
66-
const diffResult = this._differ.get({ original: this._editor.getModel()!.getValueInRange(edit.range), modified: edit.text });
67-
68-
return diffResult.promiseResult.map(p => {
69-
if (!p || !p.data) {
70-
return undefined;
71-
}
72-
const result = p.data;
73-
74-
const rangeStartPos = edit.range.getStartPosition();
75-
const innerChanges = result.changes.flatMap(c => c.innerChanges!);
76-
if (innerChanges.length === 0) {
77-
// there are no changes
78-
return undefined;
79-
}
80-
81-
function addRangeToPos(pos: Position, range: Range): Range {
82-
const start = TextLength.fromPosition(range.getStartPosition());
83-
return TextLength.ofRange(range).createRange(start.addToPosition(pos));
84-
}
85-
86-
const edits = innerChanges.map(c => new SingleTextEdit(
87-
addRangeToPos(rangeStartPos, c.originalRange),
88-
this._modifiedModel.get()!.getValueInRange(c.modifiedRange)
89-
));
90-
const diffEdits = new TextEdit(edits);
44+
const diffEdits = new TextEdit(edits);
45+
const text = new TextModelText(textModel);
9146

92-
return new InlineEditWithChanges(text, diffEdits, inlineEdit.isCollapsed, model.primaryPosition.get(), inlineEdit.renderExplicitly, inlineEdit.commands, inlineEdit.inlineCompletion); //inlineEdit.showInlineIfPossible);
93-
});
47+
return new InlineEditWithChanges(text, diffEdits, inlineEdit.isCollapsed, model.primaryPosition.get(), inlineEdit.renderExplicitly, inlineEdit.commands, inlineEdit.inlineCompletion);
9448
});
9549

96-
private readonly _inlineEdit = derivedOpts({ owner: this, equalsFn: equalsIfDefined(itemEquals()) }, reader => this._inlineEditPromise.read(reader)?.read(reader));
97-
9850
constructor(
9951
private readonly _editor: ICodeEditor,
10052
private readonly _edit: IObservable<InlineEdit | undefined>,
10153
private readonly _model: IObservable<InlineCompletionsModel | undefined>,
10254
private readonly _focusIsInMenu: ISettableObservable<boolean>,
10355
@IInstantiationService private readonly _instantiationService: IInstantiationService,
104-
@IDiffProviderFactoryService private readonly _diffProviderFactoryService: IDiffProviderFactoryService,
105-
@IModelService private readonly _modelService: IModelService
10656
) {
10757
super();
10858

0 commit comments

Comments
 (0)