Skip to content

Commit

Permalink
Inline completion refactoring (#209011)
Browse files Browse the repository at this point in the history
  • Loading branch information
hediet authored Apr 2, 2024
1 parent 7a523c2 commit 32d5ad5
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 133 deletions.
38 changes: 38 additions & 0 deletions src/vs/base/common/equals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import * as arrays from 'vs/base/common/arrays';

export type EqualityComparer<T> = (a: T, b: T) => boolean;
export const strictEquals: EqualityComparer<any> = (a, b) => a === b;

/**
* Checks if the items of two arrays are equal.
* By default, strict equality is used to compare elements, but a custom equality comparer can be provided.
*/
export function itemsEquals<T>(itemEquals: EqualityComparer<T> = strictEquals): EqualityComparer<readonly T[]> {
return (a, b) => arrays.equals(a, b, itemEquals);
}

/**
* Two items are considered equal, if their stringified representations are equal.
*/
export function jsonStringifyEquals<T>(): EqualityComparer<T> {
return (a, b) => JSON.stringify(a) === JSON.stringify(b);
}

/**
* Uses `item.equals(other)` to determine equality.
*/
export function itemEquals<T extends { equals(other: T): boolean }>(): EqualityComparer<T> {
return (a, b) => a.equals(b);
}

export function equalsIfDefined<T>(v1: T | undefined, v2: T | undefined, equals: EqualityComparer<T>): boolean {
if (!v1 || !v2) {
return v1 === v2;
}
return equals(v1, v2);
}
37 changes: 28 additions & 9 deletions src/vs/base/common/observableInternal/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { strictEquals, EqualityComparer } from 'vs/base/common/equals';
import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
import { keepObserved, recomputeInitiallyAndOnChange } from 'vs/base/common/observable';
import { DebugNameData, Owner, getFunctionName } from 'vs/base/common/observableInternal/debugName';
import { DebugNameData, IDebugNameData, Owner, getFunctionName } from 'vs/base/common/observableInternal/debugName';
import type { derivedOpts } from 'vs/base/common/observableInternal/derived';
import { getLogger } from 'vs/base/common/observableInternal/logging';

Expand Down Expand Up @@ -225,6 +226,7 @@ export abstract class ConvenientObservable<T, TChange> implements IObservable<T,
}
return undefined;
},
debugReferenceFn: fn,
},
(reader) => fn(this.read(reader), reader),
);
Expand Down Expand Up @@ -370,11 +372,26 @@ export interface ISettableObservable<T, TChange = void> extends IObservable<T, T
export function observableValue<T, TChange = void>(name: string, initialValue: T): ISettableObservable<T, TChange>;
export function observableValue<T, TChange = void>(owner: object, initialValue: T): ISettableObservable<T, TChange>;
export function observableValue<T, TChange = void>(nameOrOwner: string | object, initialValue: T): ISettableObservable<T, TChange> {
let debugNameData: DebugNameData;
if (typeof nameOrOwner === 'string') {
return new ObservableValue(undefined, nameOrOwner, initialValue);
debugNameData = new DebugNameData(undefined, nameOrOwner, undefined);
} else {
return new ObservableValue(nameOrOwner, undefined, initialValue);
debugNameData = new DebugNameData(nameOrOwner, undefined, undefined);
}
return new ObservableValue(debugNameData, initialValue, strictEquals);
}

export function observableValueOpts<T>(
options: IDebugNameData & {
equalsFn?: EqualityComparer<T>;
},
initialValue: T
): ISettableObservable<T> {
return new ObservableValue(
new DebugNameData(options.owner, options.debugName, undefined),
initialValue,
options.equalsFn ?? strictEquals,
);
}

export class ObservableValue<T, TChange = void>
Expand All @@ -383,13 +400,13 @@ export class ObservableValue<T, TChange = void>
protected _value: T;

get debugName() {
return new DebugNameData(this._owner, this._debugName, undefined).getDebugName(this) ?? 'ObservableValue';
return this._debugNameData.getDebugName(this) ?? 'ObservableValue';
}

constructor(
private readonly _owner: Owner,
private readonly _debugName: string | undefined,
private readonly _debugNameData: DebugNameData,
initialValue: T,
private readonly _equalityComparator: EqualityComparer<T>,
) {
super();
this._value = initialValue;
Expand All @@ -399,7 +416,7 @@ export class ObservableValue<T, TChange = void>
}

public set(value: T, tx: ITransaction | undefined, change: TChange): void {
if (this._value === value) {
if (this._equalityComparator(this._value, value)) {
return;
}

Expand Down Expand Up @@ -437,11 +454,13 @@ export class ObservableValue<T, TChange = void>
* When a new value is set, the previous value is disposed.
*/
export function disposableObservableValue<T extends IDisposable | undefined, TChange = void>(nameOrOwner: string | object, initialValue: T): ISettableObservable<T, TChange> & IDisposable {
let debugNameData: DebugNameData;
if (typeof nameOrOwner === 'string') {
return new DisposableObservableValue(undefined, nameOrOwner, initialValue);
debugNameData = new DebugNameData(undefined, nameOrOwner, undefined);
} else {
return new DisposableObservableValue(nameOrOwner, undefined, initialValue);
debugNameData = new DebugNameData(nameOrOwner, undefined, undefined);
}
return new DisposableObservableValue(debugNameData, initialValue, strictEquals);
}

export class DisposableObservableValue<T extends IDisposable | undefined, TChange = void> extends ObservableValue<T, TChange> implements IDisposable {
Expand Down
20 changes: 9 additions & 11 deletions src/vs/base/common/observableInternal/derived.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@
*--------------------------------------------------------------------------------------------*/

import { assertFn } from 'vs/base/common/assert';
import { strictEquals, EqualityComparer } from 'vs/base/common/equals';
import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
import { BaseObservable, IChangeContext, IObservable, IObserver, IReader, _setDerivedOpts } from 'vs/base/common/observableInternal/base';
import { BaseObservable, IChangeContext, IObservable, IObserver, IReader, _setDerivedOpts, } from 'vs/base/common/observableInternal/base';
import { DebugNameData, IDebugNameData, Owner } from 'vs/base/common/observableInternal/debugName';
import { getLogger } from 'vs/base/common/observableInternal/logging';

export type EqualityComparer<T> = (a: T, b: T) => boolean;
export const defaultEqualityComparer: EqualityComparer<any> = (a, b) => a === b;

/**
* Creates an observable that is derived from other observables.
* The value is only recomputed when absolutely needed.
Expand All @@ -28,7 +26,7 @@ export function derived<T>(computeFnOrOwner: ((reader: IReader) => T) | Owner, c
undefined,
undefined,
undefined,
defaultEqualityComparer
strictEquals
);
}
return new Derived(
Expand All @@ -37,13 +35,13 @@ export function derived<T>(computeFnOrOwner: ((reader: IReader) => T) | Owner, c
undefined,
undefined,
undefined,
defaultEqualityComparer
strictEquals
);
}

export function derivedOpts<T>(
options: IDebugNameData & {
equalityComparer?: EqualityComparer<T>;
equalsFn?: EqualityComparer<T>;
onLastObserverRemoved?: (() => void);
},
computeFn: (reader: IReader) => T
Expand All @@ -54,7 +52,7 @@ export function derivedOpts<T>(
undefined,
undefined,
options.onLastObserverRemoved,
options.equalityComparer ?? defaultEqualityComparer
options.equalsFn ?? strictEquals
);
}

Expand Down Expand Up @@ -87,7 +85,7 @@ export function derivedHandleChanges<T, TChangeSummary>(
options.createEmptyChangeSummary,
options.handleChange,
undefined,
options.equalityComparer ?? defaultEqualityComparer
options.equalityComparer ?? strictEquals
);
}

Expand All @@ -113,7 +111,7 @@ export function derivedWithStore<T>(computeFnOrOwner: ((reader: IReader, store:
}, undefined,
undefined,
() => store.dispose(),
defaultEqualityComparer
strictEquals
);
}

Expand Down Expand Up @@ -143,7 +141,7 @@ export function derivedDisposable<T extends IDisposable | undefined>(computeFnOr
}, undefined,
undefined,
() => store.dispose(),
defaultEqualityComparer
strictEquals
);
}

Expand Down
5 changes: 3 additions & 2 deletions src/vs/base/common/observableInternal/promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
*--------------------------------------------------------------------------------------------*/
import { autorun } from 'vs/base/common/observableInternal/autorun';
import { IObservable, IReader, observableValue, transaction } from './base';
import { Derived, defaultEqualityComparer, derived } from 'vs/base/common/observableInternal/derived';
import { Derived, derived } from 'vs/base/common/observableInternal/derived';
import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
import { DebugNameData, Owner } from 'vs/base/common/observableInternal/debugName';
import { strictEquals } from 'vs/base/common/equals';

export class ObservableLazy<T> {
private readonly _value = observableValue<T | undefined>(this, undefined);
Expand Down Expand Up @@ -181,6 +182,6 @@ export function derivedWithCancellationToken<T>(computeFnOrOwner: ((reader: IRea
}, undefined,
undefined,
() => cancellationTokenSource?.dispose(),
defaultEqualityComparer,
strictEquals,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class DiffEditorEditors extends Disposable {
public readonly modifiedModel = observableFromEvent(this.modified.onDidChangeModel, () => /** @description modified.model */ this.modified.getModel());

public readonly modifiedSelections = observableFromEvent(this.modified.onDidChangeCursorSelection, () => this.modified.getSelections() ?? []);
public readonly modifiedCursor = derivedOpts({ owner: this, equalityComparer: Position.equals }, reader => this.modifiedSelections.read(reader)[0]?.getPosition() ?? new Position(1, 1));
public readonly modifiedCursor = derivedOpts({ owner: this, equalsFn: Position.equals }, reader => this.modifiedSelections.read(reader)[0]?.getPosition() ?? new Position(1, 1));

public readonly originalCursor = observableFromEvent(this.original.onDidChangeCursorPosition, () => this.original.getPosition() ?? new Position(1, 1));

Expand Down
7 changes: 7 additions & 0 deletions src/vs/editor/common/core/textLength.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ export class TextLength {
return this.columnCount > other.columnCount;
}

public isGreaterThanOrEqualTo(other: TextLength): boolean {
if (this.lineCount !== other.lineCount) {
return this.lineCount > other.lineCount;
}
return this.columnCount >= other.columnCount;
}

public equals(other: TextLength): boolean {
return this.lineCount === other.lineCount && this.columnCount === other.columnCount;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class InlineCompletionsHoverParticipant implements IEditorHoverParticipan
constObservable(null),
model.selectedInlineCompletionIndex,
model.inlineCompletionsCount,
model.selectedInlineCompletion.map(v => /** @description commands */ v?.inlineCompletion.source.inlineCompletions.commands ?? []),
model.activeCommands,
);
context.fragment.appendChild(w.getDomNode());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { mapObservableArrayCached } from 'vs/base/common/observableInternal/utils';
import { ISettableObservable } from 'vs/base/common/observableInternal/base';
import { ISettableObservable, observableValueOpts } from 'vs/base/common/observableInternal/base';
import { itemsEquals, itemEquals } from 'vs/base/common/equals';

export class InlineCompletionsController extends Disposable {
static ID = 'editor.contrib.inlineCompletionsController';
Expand All @@ -41,7 +42,7 @@ export class InlineCompletionsController extends Disposable {

public readonly model = this._register(disposableObservableValue<InlineCompletionsModel | undefined>('inlineCompletionModel', undefined));
private readonly _textModelVersionId = observableValue<number, VersionIdChangeReason>(this, -1);
private readonly _positions = observableValue<readonly Position[]>(this, [new Position(1, 1)]);
private readonly _positions = observableValueOpts<readonly Position[]>({ owner: this, equalsFn: itemsEquals(itemEquals()) }, [new Position(1, 1)]);
private readonly _suggestWidgetAdaptor = this._register(new SuggestWidgetAdaptor(
this.editor,
() => this.model.get()?.selectedInlineCompletion.get()?.toSingleTextEdit(undefined),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class InlineCompletionsHintsWidget extends Disposable {
this.position,
model.selectedInlineCompletionIndex,
model.inlineCompletionsCount,
model.selectedInlineCompletion.map(v => /** @description commands */ v?.inlineCompletion.source.inlineCompletions.commands ?? []),
model.activeCommands,
));
editor.addContentWidget(contentWidget);
store.add(toDisposable(() => editor.removeContentWidget(contentWidget)));
Expand Down Expand Up @@ -151,8 +151,6 @@ export class InlineSuggestionHintsContentWidget extends Disposable implements IC
this.previousAction.enabled = this.nextAction.enabled = false;
}, 100));

private lastCommands: Command[] = [];

constructor(
private readonly editor: ICodeEditor,
private readonly withBorder: boolean,
Expand Down Expand Up @@ -225,13 +223,6 @@ export class InlineSuggestionHintsContentWidget extends Disposable implements IC
this._register(autorun(reader => {
/** @description extra commands */
const extraCommands = this._extraCommands.read(reader);
if (equals(this.lastCommands, extraCommands)) {
// nothing to update
return;
}

this.lastCommands = extraCommands;

const extraActions = extraCommands.map<IAction>(c => ({
class: undefined,
id: c.id,
Expand Down
Loading

0 comments on commit 32d5ad5

Please sign in to comment.