From 01aa2f0c7a1c201a698336f0a750916ece850238 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Tue, 7 Jun 2022 14:51:07 +0200 Subject: [PATCH 1/3] Editor manager should be singleton. Added some logging when filtering the layout data. Signed-off-by: Akos Kitta --- .../browser/arduino-ide-frontend-module.ts | 2 ++ .../theia/core/shell-layout-restorer.ts | 24 ++++++++++++++++++- .../browser/theia/editor/editor-manager.ts | 2 ++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts index 9289218b5..9ef3c2423 100644 --- a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts +++ b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts @@ -541,6 +541,8 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => { bind(SearchInWorkspaceWidget).toSelf(); rebind(TheiaSearchInWorkspaceWidget).toService(SearchInWorkspaceWidget); + // Disabled reference counter in the editor manager to avoid opening the same editor (with different opener options) multiple times. + bind(EditorManager).toSelf().inSingletonScope(); rebind(TheiaEditorManager).to(EditorManager); // replace search icon diff --git a/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts b/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts index bdf7037c7..c002b8e10 100644 --- a/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts +++ b/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts @@ -1,9 +1,16 @@ -import { injectable } from '@theia/core/shared/inversify'; +import { inject, injectable } from '@theia/core/shared/inversify'; import { FrontendApplication } from '@theia/core/lib/browser/frontend-application'; import { ShellLayoutRestorer as TheiaShellLayoutRestorer } from '@theia/core/lib/browser/shell/shell-layout-restorer'; +import { EditorManager } from '@theia/editor/lib/browser'; @injectable() export class ShellLayoutRestorer extends TheiaShellLayoutRestorer { + // The editor manager is unused in the layout restorer. + // We inject the editor manager to achieve better logging when filtering duplicate editor tabs. + // Feel free to remove it in later IDE2 releases if the duplicate editor tab issues do not occur anymore. + @inject(EditorManager) + private readonly editorManager: EditorManager; + // Workaround for https://github.com/eclipse-theia/theia/issues/6579. async storeLayoutAsync(app: FrontendApplication): Promise { if (this.shouldStoreLayout) { @@ -35,6 +42,9 @@ export class ShellLayoutRestorer extends TheiaShellLayoutRestorer { const layoutData = await this.inflate(serializedLayoutData); // workaround to remove duplicated tabs + console.log( + '>>> Filtering persisted layout data to eliminate duplicate editor tabs...' + ); const filesUri: string[] = []; if ((layoutData as any)?.mainPanel?.main?.widgets) { (layoutData as any).mainPanel.main.widgets = ( @@ -42,14 +52,26 @@ export class ShellLayoutRestorer extends TheiaShellLayoutRestorer { ).mainPanel.main.widgets.filter((widget: any) => { const uri = widget.getResourceUri().toString(); if (filesUri.includes(uri)) { + console.log(`[SKIP]: Already visited editor URI: '${uri}'.`); return false; } + console.log(`[OK]: Visited editor URI: '${uri}'.`); filesUri.push(uri); return true; }); } + console.log('<<< Filtered the layout data before restoration.'); await app.shell.setLayoutData(layoutData); + const allOpenedEditors = this.editorManager.all; + // If any editor was visited during the layout data filtering, + // but the editor manager does not know about opened editors, then + // the IDE2 will show duplicate editors. + if (filesUri.length && !allOpenedEditors.length) { + console.warn( + 'Inconsistency detected between the editor manager and the restored layout data. Editors were detected to be open in the layout data from the previous session, but the editor manager does not know about the opened editor.' + ); + } this.logger.info('<<< The layout has been successfully restored.'); return true; } diff --git a/arduino-ide-extension/src/browser/theia/editor/editor-manager.ts b/arduino-ide-extension/src/browser/theia/editor/editor-manager.ts index 245fdca3b..835715f24 100644 --- a/arduino-ide-extension/src/browser/theia/editor/editor-manager.ts +++ b/arduino-ide-extension/src/browser/theia/editor/editor-manager.ts @@ -1,5 +1,7 @@ +import { injectable } from '@theia/core/shared/inversify'; import { EditorManager as TheiaEditorManager } from '@theia/editor/lib/browser/editor-manager'; +@injectable() export class EditorManager extends TheiaEditorManager { protected override getOrCreateCounterForUri(): number { return 0; From 5505c01a685657e8a10c8364f2879d91cf26dd8d Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Tue, 7 Jun 2022 19:30:33 +0200 Subject: [PATCH 2/3] Avoid opening duplicate editor tabs. Customized the shell layout restorer: - If a resource is about to open in code editor and preview, do not open the preview. - If a resource is about to open in preview only, open a code editor instead. Signed-off-by: Akos Kitta --- .../theia/core/shell-layout-restorer.ts | 220 ++++++++++++------ 1 file changed, 152 insertions(+), 68 deletions(-) diff --git a/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts b/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts index c002b8e10..25e0f7949 100644 --- a/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts +++ b/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts @@ -1,78 +1,162 @@ -import { inject, injectable } from '@theia/core/shared/inversify'; -import { FrontendApplication } from '@theia/core/lib/browser/frontend-application'; +import { notEmpty } from '@theia/core'; +import { WidgetDescription } from '@theia/core/lib/browser'; import { ShellLayoutRestorer as TheiaShellLayoutRestorer } from '@theia/core/lib/browser/shell/shell-layout-restorer'; -import { EditorManager } from '@theia/editor/lib/browser'; +import { injectable } from '@theia/core/shared/inversify'; +import { EditorPreviewWidgetFactory } from '@theia/editor-preview/lib/browser/editor-preview-widget-factory'; +import { EditorWidgetFactory } from '@theia/editor/lib/browser/editor-widget-factory'; @injectable() export class ShellLayoutRestorer extends TheiaShellLayoutRestorer { - // The editor manager is unused in the layout restorer. - // We inject the editor manager to achieve better logging when filtering duplicate editor tabs. - // Feel free to remove it in later IDE2 releases if the duplicate editor tab issues do not occur anymore. - @inject(EditorManager) - private readonly editorManager: EditorManager; - - // Workaround for https://github.com/eclipse-theia/theia/issues/6579. - async storeLayoutAsync(app: FrontendApplication): Promise { - if (this.shouldStoreLayout) { - try { - this.logger.info('>>> Storing the layout...'); - const layoutData = app.shell.getLayoutData(); - const serializedLayoutData = this.deflate(layoutData); - await this.storageService.setData( - this.storageKey, - serializedLayoutData - ); - this.logger.info('<<< The layout has been successfully stored.'); - } catch (error) { - await this.storageService.setData(this.storageKey, undefined); - this.logger.error('Error during serialization of layout data', error); + /** + * Customized to filter out duplicate editor tabs. + */ + protected override parse( + layoutData: string, + parseContext: TheiaShellLayoutRestorer.ParseContext + ): T { + return JSON.parse(layoutData, (property: string, value) => { + if (this.isWidgetsProperty(property)) { + const widgets = parseContext.filteredArray(); + const descs = this.filterDescriptions(value); // <--- customization to filter out editor preview construction options. + for (let i = 0; i < descs.length; i++) { + parseContext.push(async (context) => { + widgets[i] = await this.convertToWidget(descs[i], context); + }); + } + return widgets; + } else if (value && typeof value === 'object' && !Array.isArray(value)) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const copy: any = {}; + for (const p in value) { + if (this.isWidgetProperty(p)) { + parseContext.push(async (context) => { + copy[p] = await this.convertToWidget(value[p], context); + }); + } else { + copy[p] = value[p]; + } + } + return copy; } - } + return value; + }); } - override async restoreLayout(app: FrontendApplication): Promise { - this.logger.info('>>> Restoring the layout state...'); - const serializedLayoutData = await this.storageService.getData( - this.storageKey - ); - if (serializedLayoutData === undefined) { - this.logger.info('<<< Nothing to restore.'); - return false; - } - - const layoutData = await this.inflate(serializedLayoutData); - // workaround to remove duplicated tabs - console.log( - '>>> Filtering persisted layout data to eliminate duplicate editor tabs...' - ); - const filesUri: string[] = []; - if ((layoutData as any)?.mainPanel?.main?.widgets) { - (layoutData as any).mainPanel.main.widgets = ( - layoutData as any - ).mainPanel.main.widgets.filter((widget: any) => { - const uri = widget.getResourceUri().toString(); - if (filesUri.includes(uri)) { - console.log(`[SKIP]: Already visited editor URI: '${uri}'.`); - return false; + /** + * Workaround to avoid duplicate editor tabs on IDE2 startup. + * + * This function filters all widget construction options with `editor-preview-widget` + * factory ID if another option has the same URI and `code-editor-opener` factory ID. + * In other words, if a resource is about to open in the Code editor, the same resource won't open as a preview widget. + * + * The other bogus state that this function is fixes when there is a resource registered to open with the `editor-preview-widget`, + * but there is no `code-editor-opener` counterpart for the same resource URI. In this case, the `editor-preview-widget` will be replaced + * with the `code-editor-opener` and the `innerWidgetState` will be dropped. + * + * OK, but why is this happening? The breaking change was [here](https://github.com/eclipse-theia/theia/commit/e8e88b76673a6151d1fc12501dbe598be2358350#diff-7e1bdbcf59009518f9f3b76fe22cc8ab82d13ffa5e5e0a4262e492f25e505d98R29-R30) + * in Theia when the editor manager of the code editor was bound to the preview editor. + * For whatever reasons, the IDE2 started to use `@theia/editor-preview` extension from [this](https://github.com/arduino/arduino-ide/commit/fc0f67493b728f9202c9a04c7243d03b0d6ea0c7) commit. From this point, when an editor was opened, + * but the `preview` option was not set to explicit `false` the preview editor manager has created the widgets instead of the manager of the regular code editor. + * This code must stay to be backward compatible. + * + * Example of resource with multiple opener: + * ```json + * [ + * { + * "constructionOptions": { + * "factoryId": "editor-preview-widget", + * "options": { + * "kind": "navigatable", + * "uri": "file:///Users/a.kitta/Documents/Arduino/sketch_jun3b/sketch_jun3b.ino", + * "counter": 1, + * "preview": false + * } + * }, + * "innerWidgetState": "{\"isPreview\":false,\"editorState\":{\"cursorState\":[{\"inSelectionMode\":false,\"selectionStart\":{\"lineNumber\":10,\"column\":1},\"position\":{\"lineNumber\":10,\"column\":1}}],\"viewState\":{\"scrollLeft\":0,\"firstPosition\":{\"lineNumber\":1,\"column\":1},\"firstPositionDeltaTop\":0},\"contributionsState\":{\"editor.contrib.folding\":{\"lineCount\":10,\"provider\":\"indent\",\"foldedImports\":false},\"editor.contrib.wordHighlighter\":false}}}" + * }, + * { + * "constructionOptions": { + * "factoryId": "code-editor-opener", + * "options": { + * "kind": "navigatable", + * "uri": "file:///Users/a.kitta/Documents/Arduino/sketch_jun3b/sketch_jun3b.ino", + * "counter": 0 + * } + * }, + * "innerWidgetState": "{\"cursorState\":[{\"inSelectionMode\":false,\"selectionStart\":{\"lineNumber\":1,\"column\":1},\"position\":{\"lineNumber\":1,\"column\":1}}],\"viewState\":{\"scrollLeft\":0,\"firstPosition\":{\"lineNumber\":1,\"column\":1},\"firstPositionDeltaTop\":0},\"contributionsState\":{\"editor.contrib.folding\":{\"lineCount\":10,\"provider\":\"indent\",\"foldedImports\":false},\"editor.contrib.wordHighlighter\":false}}" + * } + * ] + * ``` + * + * Example with resource only with preview opener: + * + * ```json + * [ + * { + * "constructionOptions": { + * "factoryId": "editor-preview-widget", + * "options": { + * "kind": "navigatable", + * "uri": "file:///c%3A/Users/per/Documents/Arduino/duptabs/duptabs.ino", + * "counter": 1, + * "preview": false + * } + * }, + * "innerWidgetState": "{\"isPreview\":false,\"editorState\":{\"cursorState\":[{\"inSelectionMode\":false,\"selectionStart\":{\"lineNumber\":1,\"column\":1},\"position\":{\"lineNumber\":1,\"column\":1}}],\"viewState\":{\"scrollLeft\":0,\"firstPosition\":{\"lineNumber\":1,\"column\":1},\"firstPositionDeltaTop\":0},\"contributionsState\":{\"editor.contrib.wordHighlighter\":false,\"editor.contrib.folding\":{\"lineCount\":11,\"provider\":\"indent\"}}}}" + * } + * ] + * ``` + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private filterDescriptions(value: any): WidgetDescription[] { + const descriptions = value as WidgetDescription[]; + const codeEditorUris = new Set(); + descriptions.forEach(({ constructionOptions }) => { + const { options, factoryId } = constructionOptions; + if (isResourceWidgetOptions(options)) { + const { uri } = options; + // resource about to open in code editor + if (factoryId === EditorWidgetFactory.ID) { + codeEditorUris.add(uri); } - console.log(`[OK]: Visited editor URI: '${uri}'.`); - filesUri.push(uri); - return true; - }); - } - console.log('<<< Filtered the layout data before restoration.'); - - await app.shell.setLayoutData(layoutData); - const allOpenedEditors = this.editorManager.all; - // If any editor was visited during the layout data filtering, - // but the editor manager does not know about opened editors, then - // the IDE2 will show duplicate editors. - if (filesUri.length && !allOpenedEditors.length) { - console.warn( - 'Inconsistency detected between the editor manager and the restored layout data. Editors were detected to be open in the layout data from the previous session, but the editor manager does not know about the opened editor.' - ); - } - this.logger.info('<<< The layout has been successfully restored.'); - return true; + } + }); + return descriptions + .map((desc) => { + const { constructionOptions } = desc; + const { options, factoryId } = constructionOptions; + if (factoryId === EditorPreviewWidgetFactory.ID) { + // resource about to open in preview editor + if (isResourceWidgetOptions(options)) { + const { uri } = options; + // if the resource is about to open in the code editor anyway, do not open the resource in a preview widget too. + if (codeEditorUris.has(uri)) { + console.log( + `Filtered a widget construction options to avoid duplicate editor tab. URI: ${options.uri}, factory ID: ${factoryId}.` + ); + return undefined; + } else { + // if the preview construction options does not have the code editor counterpart, instead of dropping the preview construction option, a code editor option will be created on the fly. + return { + constructionOptions: { + factoryId: EditorWidgetFactory.ID, + options: { + uri, + kind: 'navigatable', + counter: 0, + }, + }, + }; + } + } + } + return desc; + }) + .filter(notEmpty); } } + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function isResourceWidgetOptions(options: any): options is { uri: string } { + return !!options && 'uri' in options && typeof options.uri === 'string'; +} From 16e4623e114c8d04521db883a62bfa6afdc19e19 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 8 Jun 2022 14:26:37 +0200 Subject: [PATCH 3/3] Added logging when restoring the layout data. Signed-off-by: Akos Kitta --- .../theia/core/shell-layout-restorer.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts b/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts index 25e0f7949..23a5f9a25 100644 --- a/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts +++ b/arduino-ide-extension/src/browser/theia/core/shell-layout-restorer.ts @@ -4,9 +4,28 @@ import { ShellLayoutRestorer as TheiaShellLayoutRestorer } from '@theia/core/lib import { injectable } from '@theia/core/shared/inversify'; import { EditorPreviewWidgetFactory } from '@theia/editor-preview/lib/browser/editor-preview-widget-factory'; import { EditorWidgetFactory } from '@theia/editor/lib/browser/editor-widget-factory'; +import { FrontendApplication } from './frontend-application'; @injectable() export class ShellLayoutRestorer extends TheiaShellLayoutRestorer { + override async restoreLayout(app: FrontendApplication): Promise { + this.logger.info('>>> Restoring the layout state...'); + const serializedLayoutData = await this.storageService.getData( + this.storageKey + ); + if (serializedLayoutData === undefined) { + this.logger.info('<<< Nothing to restore.'); + return false; + } + console.log('------- SERIALIZED LAYOUT DATA -------'); + console.log(serializedLayoutData); + console.log('------- END SERIALIZED LAYOUT DATA -------'); + const layoutData = await this.inflate(serializedLayoutData); + await app.shell.setLayoutData(layoutData); + this.logger.info('<<< The layout has been successfully restored.'); + return true; + } + /** * Customized to filter out duplicate editor tabs. */