Skip to content

fix: flawed timing issue when opening editors #1649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

kittaakos merged 2 commits into main from #1612

Nov 11, 2022

Merged

fix: flawed timing issue when opening editors #1649

Changes from all commits

Commits

Show all changes

Select commit Hold shift + click to select a range

fix: flawed timing issue when opening editors

fix: workaround for arduino/arduino-cli#1968

File filter

Filter by extension

Conversations

Jump to

Jump to file

Failed to load files.

Loading

Diff view

Diff view

100 changes: 52 additions & 48 deletions arduino-ide-extension/src/browser/contributions/open-sketch-files.ts

Show comments View file Open in desktop

Original file line number	Diff line number	Diff line change
		@@ -1,5 +1,5 @@
		import { nls } from '@theia/core/lib/common/nls';
	import { inject, injectable } from '@theia/core/shared/inversify';
	import { injectable } from '@theia/core/shared/inversify';
		import type { EditorOpenerOptions } from '@theia/editor/lib/browser/editor-manager';
		import { Later } from '../../common/nls';
		import { Sketch, SketchesError } from '../../common/protocol';
Expand All		@@ -15,14 +15,9 @@ import { ApplicationError } from '@theia/core/lib/common/application-error';
		import { Deferred, wait } from '@theia/core/lib/common/promise-util';
		import { EditorWidget } from '@theia/editor/lib/browser/editor-widget';
		import { DisposableCollection } from '@theia/core/lib/common/disposable';
	import { MonacoEditor } from '@theia/monaco/lib/browser/monaco-editor';
	import { ContextKeyService as VSCodeContextKeyService } from '@theia/monaco-editor-core/esm/vs/platform/contextkey/browser/contextKeyService';

		@injectable()
		export class OpenSketchFiles extends SketchContribution {
	@inject(VSCodeContextKeyService)
	private readonly contextKeyService: VSCodeContextKeyService;

		override registerCommands(registry: CommandRegistry): void {
		registry.registerCommand(OpenSketchFiles.Commands.OPEN_SKETCH_FILES, {
		execute: (uri: URI) => this.openSketchFiles(uri),
Expand Down Expand Up		@@ -107,7 +102,7 @@ export class OpenSketchFiles extends SketchContribution {
		): Promise<Sketch \| undefined> {
		const { invalidMainSketchUri } = err.data;
		requestAnimationFrame(() => this.messageService.error(err.message));
	await wait(10); // let IDE2 toast the error message.
	await wait(250); // let IDE2 open the editor and toast the error message, then open the modal dialog
		const movedSketch = await promptMoveSketch(invalidMainSketchUri, {
		fileService: this.fileService,
		sketchService: this.sketchService,
Expand Down Expand Up		@@ -135,39 +130,36 @@ export class OpenSketchFiles extends SketchContribution {
		const widget = this.editorManager.all.find(
		(widget) => widget.editor.uri.toString() === uri
		);
	if (widget && !forceOpen) {
	return widget;
	}

		const disposables = new DisposableCollection();
	if (!widget \|\| forceOpen) {
	const deferred = new Deferred<EditorWidget>();
	const deferred = new Deferred<EditorWidget>();
	// An editor can be in two primary states:
	// - The editor is not yet opened. The `widget` is `undefined`. With `editorManager#open`, Theia will create an editor and fire an `editorManager#onCreated` event.
	// - The editor is opened. Can be active, current, or open.
	// - If the editor has the focus (the cursor blinks in the editor): it's the active editor.
	// - If the editor does not have the focus (the focus is on a different widget or the context menu is opened in the editor): it's the current editor.
	// - If the editor is not the top editor in the main area, it's opened.
	if (!widget) {
	// If the widget is `undefined`, IDE2 expects one `onCreate` event. Subscribe to the `onCreated` event
	// and resolve the promise with the editor only when the new editor's visibility changes.
		disposables.push(
		this.editorManager.onCreated((editor) => {
		if (editor.editor.uri.toString() === uri) {
	if (editor.isVisible) {
	disposables.dispose();
	if (editor.isAttached && editor.isVisible) {
		deferred.resolve(editor);
		} else {
	// In Theia, the promise resolves after opening the editor, but the editor is neither attached to the DOM, nor visible.
	// This is a hack to first get an event from monaco after the widget update request, then IDE2 waits for the next monaco context key event.
	// Here, the monaco context key event is not used, but this is the first event after the editor is visible in the UI.
		disposables.push(
	(editor.editor as MonacoEditor).onDidResize((dimension) => {
	if (dimension) {
	const isKeyOwner = (
	arg: unknown
	): arg is { key: string } => {
	if (typeof arg === 'object') {
	const object = arg as Record<string, unknown>;
	return typeof object['key'] === 'string';
	}
	return false;
	};
	disposables.push(
	this.contextKeyService.onDidChangeContext((e) => {
	// `commentIsEmpty` is the first context key change event received from monaco after the editor is for real visible in the UI.
	if (isKeyOwner(e) && e.key === 'commentIsEmpty') {
	deferred.resolve(editor);
	disposables.dispose();
	}
	})
	editor.onDidChangeVisibility((visible) => {
	if (visible) {
	// wait an animation frame. although the visible and attached props are true the editor is not there.
	// let the browser render the widget
	setTimeout(
	() =>
	requestAnimationFrame(() => deferred.resolve(editor)),
	0
		);
		}
		})
Expand All		@@ -176,29 +168,41 @@ export class OpenSketchFiles extends SketchContribution {
		}
		})
		);
	this.editorManager.open(
	}

	this.editorManager
	.open(
		new URI(uri),
		options ?? {
		mode: 'reveal',
		preview: false,
		counter: 0,
		}
	)
	.then((editorWidget) => {
	// If the widget was defined, it was already opened.
	// The editor is expected to be attached to the shell and visible in the UI.
	// The deferred promise does not have to wait for the `editorManager#onCreated` event.
	// It can resolve earlier.
	if (!widget) {
	deferred.resolve(editorWidget);
	}
	});

	const timeout = 5_000; // number of ms IDE2 waits for the editor to show up in the UI
	const result = await Promise.race([
	deferred.promise,
	wait(timeout).then(() => {
	disposables.dispose();
	return 'timeout';
	}),
	]);
	if (result === 'timeout') {
	console.warn(
	`Timeout after ${timeout} millis. The editor has not shown up in time. URI: ${uri}`
		);
	const timeout = 5_000; // number of ms IDE2 waits for the editor to show up in the UI
	const result = await Promise.race([
	deferred.promise,
	wait(timeout).then(() => {
	disposables.dispose();
	return 'timeout';
	}),
	]);
	if (result === 'timeout') {
	console.warn(
	`Timeout after ${timeout} millis. The editor has not shown up in time. URI: ${uri}`
	);
	}
	return result;
		}
	return result;
		}
		}
		export namespace OpenSketchFiles {
Expand Down

58 changes: 11 additions & 47 deletions arduino-ide-extension/src/electron-main/theia/electron-main-application.ts

Show comments View file Open in desktop

Original file line number	Diff line number	Diff line change
Expand Up		@@ -9,7 +9,7 @@ import {
		import { fork } from 'child_process';
		import { AddressInfo } from 'net';
		import { join, isAbsolute, resolve } from 'path';
	import { promises as fs, Stats } from 'fs';
	import { promises as fs } from 'fs';
		import { MaybePromise } from '@theia/core/lib/common/types';
		import { ElectronSecurityToken } from '@theia/core/lib/electron-common/electron-token';
		import { FrontendApplicationConfig } from '@theia/application-package/lib/application-props';
Expand All		@@ -28,6 +28,7 @@ import {
		SHOW_PLOTTER_WINDOW,
		} from '../../common/ipc-communication';
		import { ErrnoException } from '../../node/utils/errors';
	import { isAccessibleSketchPath } from '../../node/sketches-service-impl';

		app.commandLine.appendSwitch('disable-http-cache');

Expand Down Expand Up		@@ -145,7 +146,10 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
		event.preventDefault();
		const resolvedPath = await this.resolvePath(path, cwd);
		if (resolvedPath) {
	const sketchFolderPath = await this.isValidSketchPath(resolvedPath);
	const sketchFolderPath = await isAccessibleSketchPath(
	resolvedPath,
	true
	);
		if (sketchFolderPath) {
		this.openFilePromise.reject(new InterruptWorkspaceRestoreError());
		await this.openSketch(sketchFolderPath);
Expand All		@@ -158,49 +162,6 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
		}
		}

	/**
	* The `path` argument is valid, if accessible and either pointing to a `.ino` file,
	* or it's a directory, and one of the files in the directory is an `.ino` file.
	*
	* If `undefined`, `path` was pointing to neither an accessible sketch file nor a sketch folder.
	*
	* The sketch folder name and sketch file name can be different. This method is not sketch folder name compliant.
	* The `path` must be an absolute, resolved path.
	*/
	private async isValidSketchPath(path: string): Promise<string \| undefined> {
	let stats: Stats \| undefined = undefined;
	try {
	stats = await fs.stat(path);
	} catch (err) {
	if (ErrnoException.isENOENT(err)) {
	return undefined;
	}
	throw err;
	}
	if (!stats) {
	return undefined;
	}
	if (stats.isFile()) {
	return path.endsWith('.ino') ? path : undefined;
	}
	try {
	const entries = await fs.readdir(path, { withFileTypes: true });
	const sketchFilename = entries
	.filter((entry) => entry.isFile() && entry.name.endsWith('.ino'))
	.map(({ name }) => name)
	.sort((left, right) => left.localeCompare(right))[0];
	if (sketchFilename) {
	return join(path, sketchFilename);
	}
	// If no sketches found in the folder, but the folder exists,
	// return with the path of the empty folder and let IDE2's frontend
	// figure out the workspace root.
	return path;
	} catch (err) {
	throw err;
	}
	}

		private async resolvePath(
		maybePath: string,
		cwd: string
Expand Down Expand Up		@@ -253,7 +214,10 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
		if (!resolvedPath) {
		continue;
		}
	const sketchFolderPath = await this.isValidSketchPath(resolvedPath);
	const sketchFolderPath = await isAccessibleSketchPath(
	resolvedPath,
	true
	);
		if (sketchFolderPath) {
		workspace.file = sketchFolderPath;
		if (this.isTempSketch.is(workspace.file)) {
Expand Down Expand Up		@@ -284,7 +248,7 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
		if (!resolvedPath) {
		continue;
		}
	const sketchFolderPath = await this.isValidSketchPath(resolvedPath);
	const sketchFolderPath = await isAccessibleSketchPath(resolvedPath, true);
		if (sketchFolderPath) {
		path = sketchFolderPath;
		break;
Expand Down

100 changes: 53 additions & 47 deletions arduino-ide-extension/src/node/sketches-service-impl.ts

Show comments View file Open in desktop

Original file line number	Diff line number	Diff line change
Expand Up		@@ -734,62 +734,68 @@ function isNotFoundError(err: unknown): err is ServiceError {

		/**
		* Tries to detect whether the error was caused by an invalid main sketch file name.
	* IDE2 should handle gracefully when there is an invalid sketch folder name. See the [spec](https://arduino.github.io/arduino-cli/latest/sketch-specification/#sketch-root-folder) for details.
	* The CLI does not have error codes (https://github.com/arduino/arduino-cli/issues/1762), so IDE2 parses the error message and tries to guess it.
	* IDE2 should handle gracefully when there is an invalid sketch folder name.
	* See the [spec](https://arduino.github.io/arduino-cli/latest/sketch-specification/#sketch-root-folder) for details.
	* The CLI does not have error codes (https://github.com/arduino/arduino-cli/issues/1762),
	* IDE2 cannot parse the error message (https://github.com/arduino/arduino-cli/issues/1968#issuecomment-1306936142)
	* so it checks if a sketch even if it's invalid can be discovered from the requested path.
		* Nothing guarantees that the invalid existing main sketch file still exits by the time client performs the sketch move.
		*/
		async function isInvalidSketchNameError(
		cliErr: unknown,
		requestSketchPath: string
		): Promise<string \| undefined> {
	if (isNotFoundError(cliErr)) {
	const ino = requestSketchPath.endsWith('.ino');
	if (ino) {
	const sketchFolderPath = path.dirname(requestSketchPath);
	const sketchName = path.basename(sketchFolderPath);
	const pattern = escapeRegExpCharacters(
	`${invalidSketchNameErrorRegExpPrefix}${path.join(
	sketchFolderPath,
	`${sketchName}.ino`
	)}`
	);
	if (new RegExp(pattern, 'i').test(cliErr.details)) {
	try {
	await fs.access(requestSketchPath);
	return requestSketchPath;
	} catch {
	return undefined;
	}
	}
	} else {
	try {
	const resources = await fs.readdir(requestSketchPath, {
	withFileTypes: true,
	});
	return (
	resources
	.filter((resource) => resource.isFile())
	.filter((resource) => resource.name.endsWith('.ino'))
	// A folder might contain multiple sketches. It's OK to ick the first one as IDE2 cannot do much,
	// but ensure a deterministic behavior as `readdir(3)` does not guarantee an order. Sort them.
	.sort(({ name: left }, { name: right }) =>
	left.localeCompare(right)
	)
	.map(({ name }) => name)
	.map((name) => path.join(requestSketchPath, name))[0]
	);
	} catch (err) {
	if (ErrnoException.isENOENT(err) \|\| ErrnoException.isENOTDIR(err)) {
	return undefined;
	}
	throw err;
	}
	return isNotFoundError(cliErr)
	? isAccessibleSketchPath(requestSketchPath)
	: undefined;
	}

	/**
	* The `path` argument is valid, if accessible and either pointing to a `.ino` file,
	* or it's a directory, and one of the files in the directory is an `.ino` file.
	*
	* `undefined` if `path` was pointing to neither an accessible sketch file nor a sketch folder.
	*
	* The sketch folder name and sketch file name can be different. This method is not sketch folder name compliant.
	* The `path` must be an absolute, resolved path. This method does not handle EACCES (Permission denied) errors.
	*
	* When `fallbackToInvalidFolderPath` is `true`, and the `path` is an accessible folder without any sketch files,
	* this method returns with the `path` argument instead of `undefined`.
	*/
	export async function isAccessibleSketchPath(
	path: string,
	fallbackToInvalidFolderPath = false
	): Promise<string \| undefined> {
	let stats: Stats \| undefined = undefined;
	try {
	stats = await fs.stat(path);
	} catch (err) {
	if (ErrnoException.isENOENT(err)) {
	return undefined;
		}
	throw err;
	}
	if (!stats) {
	return undefined;
	}
	if (stats.isFile()) {
	return path.endsWith('.ino') ? path : undefined;
	}
	const entries = await fs.readdir(path, { withFileTypes: true });
	const sketchFilename = entries
	.filter((entry) => entry.isFile() && entry.name.endsWith('.ino'))
	.map(({ name }) => name)
	// A folder might contain multiple sketches. It's OK to pick the first one as IDE2 cannot do much,
	// but ensure a deterministic behavior as `readdir(3)` does not guarantee an order. Sort them.
	.sort((left, right) => left.localeCompare(right))[0];
	if (sketchFilename) {
	return join(path, sketchFilename);
		}
	return undefined;
	// If no sketches found in the folder, but the folder exists,
	// return with the path of the empty folder and let IDE2's frontend
	// figure out the workspace root.
	return fallbackToInvalidFolderPath ? path : undefined;
		}
	const invalidSketchNameErrorRegExpPrefix =
	'.*: main file missing from sketch: ';

		/*
		* When a new sketch is created, add a suffix to distinguish it
Expand Down