Skip to content

Commit 00375ba

Browse files
committed
Give up on transforming in compose step, do it afterwards
in order to avoid transformation timeout
1 parent 7a0b84f commit 00375ba

File tree

2 files changed

+49
-56
lines changed

2 files changed

+49
-56
lines changed

packages/jupyterlab-lsp/src/index.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ export class LSPExtension implements ILSPExtension {
154154
connection_manager: DocumentConnectionManager;
155155
language_server_manager: LanguageServerManager;
156156
feature_manager: ILSPFeatureManager;
157+
private _settingsUI: SettingsUIManager;
157158

158159
constructor(
159160
public app: JupyterFrontEnd,
@@ -200,15 +201,15 @@ export class LSPExtension implements ILSPExtension {
200201

201202
this.feature_manager = new FeatureManager();
202203

203-
const settingsUI = new SettingsUIManager({
204+
this._settingsUI = new SettingsUIManager({
204205
settingRegistry: this.setting_registry,
205206
formRegistry: formRegistry,
206207
console: this.console.scope('SettingsUIManager'),
207208
languageServerManager: this.language_server_manager,
208209
trans: trans,
209210
restored: app.restored
210211
});
211-
settingsUI
212+
this._settingsUI
212213
.setupSchemaForUI(plugin.id)
213214
.then(this._activate.bind(this))
214215
.catch(this._activate.bind(this));
@@ -218,7 +219,9 @@ export class LSPExtension implements ILSPExtension {
218219
this.setting_registry
219220
.load(plugin.id)
220221
.then(settings => {
221-
const options = settings.composite as Required<LanguageServer>;
222+
const options = this._settingsUI.normalizeSettings(
223+
settings.composite as Required<LanguageServer>
224+
);
222225

223226
// Store the initial server settings, to be sent asynchronously
224227
// when the servers are initialized.
@@ -269,7 +272,9 @@ export class LSPExtension implements ILSPExtension {
269272
}
270273

271274
private updateOptions(settings: ISettingRegistry.ISettings) {
272-
const options = settings.composite as Required<LanguageServer>;
275+
const options = this._settingsUI.normalizeSettings(
276+
settings.composite as Required<LanguageServer>
277+
);
273278

274279
const languageServerSettings = (options.language_servers ||
275280
{}) as TLanguageServerConfigurations;

packages/jupyterlab-lsp/src/settings.ts

Lines changed: 40 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -278,58 +278,6 @@ export class SettingsUIManager {
278278
: this._canonical,
279279
version: plugin.version
280280
};
281-
},
282-
compose: plugin => {
283-
// Initial compose: 28 ms
284-
// Consecutive compose with cached settings: 1-2ms
285-
286-
const user = plugin.data.user as Required<LanguageServer>;
287-
const composite = JSONExt.deepCopy(user);
288-
289-
// Cache collapsed settings for speed and to only show dialog once.
290-
// Note that JupyterLab attempts to transform in "preload" step (before splash screen end)
291-
// and then again for deferred extensions if the initial transform in preload timed out.
292-
// We are hitting the timeout in preload step.
293-
if (
294-
this._lastUserServerSettings === null ||
295-
this._lastUserServerSettingsDoted === null ||
296-
!JSONExt.deepEqual(
297-
this._lastUserServerSettings,
298-
user.language_servers
299-
)
300-
) {
301-
this._lastUserServerSettings = user.language_servers;
302-
const collapsed = this._collapseServerSettingsDotted(
303-
user.language_servers
304-
);
305-
user.language_servers = this._collapseServerSettingsDotted(
306-
user.language_servers
307-
).settings;
308-
this._lastUserServerSettingsDoted = user.language_servers;
309-
310-
if (Object.keys(collapsed.conflicts).length > 0) {
311-
this._warnConflicts(collapsed.conflicts).catch(this.console.warn);
312-
}
313-
} else {
314-
user.language_servers = this._lastUserServerSettingsDoted;
315-
}
316-
composite.language_servers = user.language_servers;
317-
318-
// Currently disabled, as it does not provide an obvious benefit:
319-
// - we would need to explicitly save the updated settings
320-
// to get a clean version in JSON Setting Editor.
321-
// - if default changed on the server side but schema did not get
322-
// updated, server would be using a different value than communicated
323-
// to the user. It would be optimal to filter out defaults from
324-
// user data and always keep them in composite.
325-
// user.language_servers = this._filterOutDefaults(user.language_servers);
326-
327-
plugin.data = {
328-
user: user,
329-
composite: composite
330-
};
331-
332-
return plugin;
333281
}
334282
});
335283

@@ -341,6 +289,46 @@ export class SettingsUIManager {
341289
});
342290
}
343291

292+
normalizeSettings(composite: Required<LanguageServer>) {
293+
// Cache collapsed settings for speed and to only show dialog once.
294+
// Note that JupyterLab attempts to transform in "preload" step (before splash screen end)
295+
// and then again for deferred extensions if the initial transform in preload timed out.
296+
// We are hitting the timeout in preload step.
297+
if (
298+
this._lastUserServerSettings === null ||
299+
this._lastUserServerSettingsDoted === null ||
300+
!JSONExt.deepEqual(
301+
this._lastUserServerSettings,
302+
composite.language_servers
303+
)
304+
) {
305+
this._lastUserServerSettings = composite.language_servers;
306+
const collapsed = this._collapseServerSettingsDotted(
307+
composite.language_servers
308+
);
309+
composite.language_servers = collapsed.settings;
310+
this._lastUserServerSettingsDoted = composite.language_servers;
311+
312+
if (Object.keys(collapsed.conflicts).length > 0) {
313+
this._warnConflicts(collapsed.conflicts).catch(this.console.warn);
314+
}
315+
} else {
316+
composite.language_servers = this._lastUserServerSettingsDoted;
317+
}
318+
319+
// Currently disabled, as it does not provide an obvious benefit:
320+
// - we would need to explicitly save the updated settings
321+
// to get a clean version in JSON Setting Editor.
322+
// - if default changed on the server side but schema did not get
323+
// updated, server would be using a different value than communicated
324+
// to the user. It would be optimal to filter out defaults from
325+
// user data and always keep them in composite.
326+
// composite.language_servers = this._filterOutDefaults(composite.language_servers);
327+
328+
// TODO: trigger update of settings to ensure that UI uses the same settings as collapsed?
329+
return composite;
330+
}
331+
344332
private _wasPreviouslyValidated(
345333
plugin: ISettingRegistry.IPlugin,
346334
schema: ISettingRegistry.ISchema

0 commit comments

Comments
 (0)