Skip to content

Commit c75dd82

Browse files
hediethediet
authored
Improves observable logging (microsoft#189355)
1 parent e2a0fe7 commit c75dd82

File tree

10 files changed

+63
-30
lines changed

10 files changed

+63
-30
lines changed

‎src/vs/base/common/observableInternal/autorun.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ export class AutorunObserver<TChangeSummary = any> implements IObserver, IReader
144144
this.runFn(this, changeSummary);
145145
}
146146
} finally {
147+
getLogger()?.handleAutorunFinished(this);
147148
// We don't want our observed observables to think that they are (not even temporarily) not being observed.
148149
// Thus, we only unsubscribe from observables that are definitely not read anymore.
149150
for (const o of this.dependenciesToBeRemoved) {

‎src/vs/base/common/observableInternal/base.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,17 @@ export abstract class ConvenientObservable<T, TChange> implements IObservable<T,
165165
(reader) => fn(this.read(reader), reader),
166166
() => {
167167
const name = getFunctionName(fn);
168-
return name !== undefined ? name : `${this.debugName} (mapped)`;
168+
if (name !== undefined) {
169+
return name;
170+
}
171+
172+
// regexp to match `x => x.y` where x and y can be arbitrary identifiers (uses backref):
173+
const regexp = /^\s*\(?\s*([a-zA-Z_$][a-zA-Z_$0-9]*)\s*\)?\s*=>\s*\1\.([a-zA-Z_$][a-zA-Z_$0-9]*)\s*$/;
174+
const match = regexp.exec(fn.toString());
175+
if (match) {
176+
return `${this.debugName}.${match[2]}`;
177+
}
178+
return `${this.debugName} (mapped)`;
169179
},
170180
);
171181
}
@@ -198,11 +208,9 @@ export abstract class BaseObservable<T, TChange = void> extends ConvenientObserv
198208
export function transaction(fn: (tx: ITransaction) => void, getDebugName?: () => string): void {
199209
const tx = new TransactionImpl(fn, getDebugName);
200210
try {
201-
getLogger()?.handleBeginTransaction(tx);
202211
fn(tx);
203212
} finally {
204213
tx.finish();
205-
getLogger()?.handleEndTransaction();
206214
}
207215
}
208216

@@ -217,7 +225,9 @@ export function subtransaction(tx: ITransaction | undefined, fn: (tx: ITransacti
217225
export class TransactionImpl implements ITransaction {
218226
private updatingObservers: { observer: IObserver; observable: IObservable<any> }[] | null = [];
219227

220-
constructor(private readonly fn: Function, private readonly _getDebugName?: () => string) { }
228+
constructor(private readonly fn: Function, private readonly _getDebugName?: () => string) {
229+
getLogger()?.handleBeginTransaction(this);
230+
}
221231

222232
public getDebugName(): string | undefined {
223233
if (this._getDebugName) {
@@ -238,6 +248,7 @@ export class TransactionImpl implements ITransaction {
238248
for (const { observer, observable } of updatingObservers) {
239249
observer.endUpdate(observable);
240250
}
251+
getLogger()?.handleEndTransaction();
241252
}
242253
}
243254

@@ -287,7 +298,7 @@ export class ObservableValue<T, TChange = void>
287298
try {
288299
const oldValue = this._value;
289300
this._setValue(value);
290-
getLogger()?.handleObservableChanged(this, { oldValue, newValue: value, change, didChange: true });
301+
getLogger()?.handleObservableChanged(this, { oldValue, newValue: value, change, didChange: true, hadValue: true });
291302

292303
for (const observer of this.observers) {
293304
tx.updateObserver(observer, this);

‎src/vs/base/common/observableInternal/derived.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ export class Derived<T, TChangeSummary = any> extends BaseObservable<T, void> im
169169
oldValue,
170170
newValue: this.value,
171171
change: undefined,
172-
didChange
172+
didChange,
173+
hadValue,
173174
});
174175

175176
if (didChange) {

‎src/vs/base/common/observableInternal/logging.ts

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,19 @@ interface IChangeInformation {
2323
newValue: unknown;
2424
change: unknown;
2525
didChange: boolean;
26+
hadValue: boolean;
2627
}
2728

2829
export interface IObservableLogger {
29-
handleObservableChanged(observable: ObservableValue<unknown, unknown>, info: IChangeInformation): void;
30+
handleObservableChanged(observable: ObservableValue<any, any>, info: IChangeInformation): void;
3031
handleFromEventObservableTriggered(observable: FromEventObservable<any, any>, info: IChangeInformation): void;
3132

3233
handleAutorunCreated(autorun: AutorunObserver): void;
3334
handleAutorunTriggered(autorun: AutorunObserver): void;
35+
handleAutorunFinished(autorun: AutorunObserver): void;
3436

35-
handleDerivedCreated(observable: Derived<unknown>): void;
36-
handleDerivedRecomputed(observable: Derived<unknown>, info: IChangeInformation): void;
37+
handleDerivedCreated(observable: Derived<any>): void;
38+
handleDerivedRecomputed(observable: Derived<any>, info: IChangeInformation): void;
3739

3840
handleBeginTransaction(transaction: TransactionImpl): void;
3941
handleEndTransaction(): void;
@@ -50,6 +52,15 @@ export class ConsoleObservableLogger implements IObservableLogger {
5052
}
5153

5254
private formatInfo(info: IChangeInformation): ConsoleText[] {
55+
if (!info.hadValue) {
56+
return [
57+
normalText(` `),
58+
styled(formatValue(info.newValue, 60), {
59+
color: 'green',
60+
}),
61+
normalText(` (initial)`),
62+
];
63+
}
5364
return info.didChange
5465
? [
5566
normalText(` `),
@@ -102,7 +113,8 @@ export class ConsoleObservableLogger implements IObservableLogger {
102113
formatKind('derived recomputed'),
103114
styled(derived.debugName, { color: 'BlueViolet' }),
104115
...this.formatInfo(info),
105-
this.formatChanges(changedObservables)
116+
this.formatChanges(changedObservables),
117+
{ data: [{ fn: derived['computeFn'] }] }
106118
]));
107119
changedObservables.clear();
108120
}
@@ -112,6 +124,7 @@ export class ConsoleObservableLogger implements IObservableLogger {
112124
formatKind('observable from event triggered'),
113125
styled(observable.debugName, { color: 'BlueViolet' }),
114126
...this.formatInfo(info),
127+
{ data: [{ fn: observable['getValue'] }] }
115128
]));
116129
}
117130

@@ -129,9 +142,15 @@ export class ConsoleObservableLogger implements IObservableLogger {
129142
console.log(...this.textToConsoleArgs([
130143
formatKind('autorun'),
131144
styled(autorun.debugName, { color: 'BlueViolet' }),
132-
this.formatChanges(changedObservables)
145+
this.formatChanges(changedObservables),
146+
{ data: [{ fn: autorun['runFn'] }] }
133147
]));
134148
changedObservables.clear();
149+
this.indentation++;
150+
}
151+
152+
handleAutorunFinished(autorun: AutorunObserver): void {
153+
this.indentation--;
135154
}
136155

137156
handleBeginTransaction(transaction: TransactionImpl): void {
@@ -142,6 +161,7 @@ export class ConsoleObservableLogger implements IObservableLogger {
142161
console.log(...this.textToConsoleArgs([
143162
formatKind('transaction'),
144163
styled(transactionName, { color: 'BlueViolet' }),
164+
{ data: [{ fn: transaction['fn'] }] }
145165
]));
146166
this.indentation++;
147167
}
@@ -153,13 +173,12 @@ export class ConsoleObservableLogger implements IObservableLogger {
153173

154174
type ConsoleText =
155175
| (ConsoleText | undefined)[]
156-
| { text: string; style: string; data?: Record<string, unknown> }
157-
| { data: Record<string, unknown> };
176+
| { text: string; style: string; data?: unknown[] }
177+
| { data: unknown[] };
158178

159179
function consoleTextToArgs(text: ConsoleText): unknown[] {
160180
const styles = new Array<any>();
161-
const initial = {};
162-
const data = initial;
181+
const data: unknown[] = [];
163182
let firstArg = '';
164183

165184
function process(t: ConsoleText): void {
@@ -173,20 +192,17 @@ function consoleTextToArgs(text: ConsoleText): unknown[] {
173192
firstArg += `%c${t.text}`;
174193
styles.push(t.style);
175194
if (t.data) {
176-
Object.assign(data, t.data);
195+
data.push(...t.data);
177196
}
178197
} else if ('data' in t) {
179-
Object.assign(data, t.data);
198+
data.push(...t.data);
180199
}
181200
}
182201

183202
process(text);
184203

185204
const result = [firstArg, ...styles];
186-
if (Object.keys(data).length > 0) {
187-
result.push(data);
188-
}
189-
205+
result.push(...data);
190206
return result;
191207
}
192208

‎src/vs/base/common/observableInternal/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export class FromEventObservable<TArgs, T> extends BaseObservable<T> {
112112

113113
const didChange = !this.hasValue || this.value !== newValue;
114114

115-
getLogger()?.handleFromEventObservableTriggered(this, { oldValue: this.value, newValue, change: undefined, didChange });
115+
getLogger()?.handleFromEventObservableTriggered(this, { oldValue: this.value, newValue, change: undefined, didChange, hadValue: this.hasValue });
116116

117117
if (didChange) {
118118
this.value = newValue;

‎src/vs/editor/browser/widget/diffEditorWidget2/lineAlignment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ export class ViewZoneManager extends Disposable {
405405

406406

407407
this._register(autorun(reader => {
408-
/** @description update */
408+
/** @description update editor top offsets */
409409
const m = this._diffModel.read(reader)?.syncedMovedTexts.read(reader);
410410

411411
let deltaOrigToMod = 0;

‎src/vs/editor/browser/widget/diffEditorWidget2/movedBlocksLines.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export class MovedBlocksLinesPart extends Disposable {
2828
this._rootElement.appendChild(element);
2929

3030
this._register(autorun(reader => {
31-
/** @description update */
31+
/** @description update moved blocks lines positioning */
3232
const info = this._originalEditorLayoutInfo.read(reader);
3333
const info2 = this._modifiedEditorLayoutInfo.read(reader);
3434
if (!info || !info2) {
@@ -45,8 +45,14 @@ export class MovedBlocksLinesPart extends Disposable {
4545
const viewZonesChanged = observableSignalFromEvent('onDidChangeViewZones', this._editors.modified.onDidChangeViewZones);
4646

4747
this._register(autorun(reader => {
48-
/** @description update */
4948
element.replaceChildren();
49+
50+
/** @description update moved blocks lines */
51+
const moves = this._diffModel.read(reader)?.diff.read(reader)?.movedTexts;
52+
if (!moves) {
53+
return;
54+
}
55+
5056
viewZonesChanged.read(reader);
5157

5258
const info = this._originalEditorLayoutInfo.read(reader);
@@ -56,11 +62,6 @@ export class MovedBlocksLinesPart extends Disposable {
5662
}
5763
const width = info.verticalScrollbarWidth + info.contentLeft - MovedBlocksLinesPart.movedCodeBlockPadding;
5864

59-
const moves = this._diffModel.read(reader)?.diff.read(reader)?.movedTexts;
60-
if (!moves) {
61-
return;
62-
}
63-
6465
let idx = 0;
6566
for (const m of moves) {
6667
function computeLineStart(range: LineRange, editor: ICodeEditor) {

‎src/vs/editor/browser/widget/diffEditorWidget2/utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ export class ObservableElementSizeObserver extends Disposable {
102102
this._height = observableValue('height', this.elementSizeObserver.getHeight());
103103

104104
this._register(this.elementSizeObserver.onDidChange(e => transaction(tx => {
105+
/** @description Set width/height from elementSizeObserver */
105106
this._width.set(this.elementSizeObserver.getWidth(), tx);
106107
this._height.set(this.elementSizeObserver.getHeight(), tx);
107108
})));

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export class InlineCompletionsSource extends Disposable {
9090

9191
this._updateOperation.clear();
9292
transaction(tx => {
93+
/** @description Update completions with provider result */
9394
target.set(completions, tx);
9495
});
9596

‎src/vs/editor/contrib/inlineCompletions/browser/suggestWidgetInlineCompletionProvider.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ export class SuggestWidgetAdaptor extends Disposable {
123123
this._currentSuggestItemInfo = newInlineCompletion;
124124

125125
transaction(tx => {
126+
/** @description Update state from suggest widget */
126127
this.checkModelVersion(tx);
127128
this._selectedItem.set(this._isActive ? this._currentSuggestItemInfo : undefined, tx);
128129
});

0 commit comments

Comments
 (0)