rework monaco commands to align with VS Code

[akosyakov]

What it does

• Reverts 40b1d45 to align with VS Code behaviour as before
• Refactors monaco-commands to fix Find/Replace/Undo/Redo/Select All, but at the same time without breaking previous behaviour:

  theia/packages/monaco/src/browser/monaco-command.ts

  Lines 80 to 187 in 42ea5bd

  	/**
  	* Register commands from Monaco to Theia registry.
  	*
  	* Monaco has different kind of commands which should be handled differently by Theia.
  	*
  	* ### Editor Actions
  	*
  	* They should be registered with a label to be visible in the quick command palette.
  	*
  	* Such actions should be enabled only if the current editor is available and
  	* it supports such action in the current context.
  	*
  	* ### Editor Commands
  	*
  	* Such actions should be enabled only if the current editor is available.
  	*
  	* `actions.find` and `editor.action.startFindReplaceAction` are registed as handlers for `find` and `replace`.
  	* If handlers are not enabled then the core should prevent the default browser behaviour.
  	* Other Theia extensions can register alternative implementations using custom enablement.
  	*
  	* ### Global Commands
  	*
  	* These commands are not necessary dependend on the current editor and enabled always.
  	* But they depend on services which are global in VS Code, but bound to the editor in Monaco,
  	* i.e. `ICodeEditorService` or `IContextKeyService`. We should take care of providing Theia implementations for such services.
  	*
  	* #### Global Native or Editor Commands
  	*
  	* Namely: `undo`, `redo` and `editor.action.selectAll`. They depend on `ICodeEditorService`.
  	* They will try to delegate to the current editor and if it is not available delegate to the browser.
  	* They are registered as handlers for corresponding core commands always.
  	* Other Theia extensions can provide alternative implementations by introducing a dependency to `@theia/monaco` extension.
  	*
  	* #### Global Language Commands
  	*
  	* Like `_executeCodeActionProvider`, they depend on `ICodeEditorService` and `ITextModelService`.
  	*
  	* #### Global Context Commands
  	*
  	* It is `setContext`. It depends on `IContextKeyService`.
  	*
  	* #### Global Editor Commands
  	*
  	* Like `openReferenceToSide` and `openReference`, they depend on `IListService`.
  	* We treat all commands which don't match any other category of global commands as global editor commands
  	* and execute them using the instantiation service of the current editor.
  	*/
  	protected registerMonacoCommands(): void {
  	const editorRegistry = monaco.editorExtensions.EditorExtensionsRegistry;
  	const editorActions = new Map(editorRegistry.getEditorActions().map(({ id, label }) => [id, label]));
  	const { codeEditorService, textModelService, contextKeyService } = this;
  	const [, globalInstantiationService] = monaco.services.StaticServices.init({ codeEditorService, textModelService, contextKeyService });
  	const monacoCommands = monaco.commands.CommandsRegistry.getCommands();
  	for (const id of monacoCommands.keys()) {
  	if (MonacoCommands.EXCLUDE_ACTIONS.has(id)) {
  	continue;
  	}
  	const handler: CommandHandler = {
  	execute: (...args) => {
  	const editor = codeEditorService.getActiveCodeEditor();
  	if (editorActions.has(id)) {
  	const action = editor && editor.getAction(id);
  	if (!action) {
  	return;
  	}
  	return action.run();
  	}
  	if (editorRegistry.getEditorCommand(id)) {
  	if (!editor) {
  	return;
  	}
  	return (editor._commandService as MonacoCommandService).executeMonacoCommand(id, ...args);
  	}
  	let instantiationService;
  	if (id.startsWith('_execute') || id === 'setContext' || MonacoCommands.COMMON_ACTIONS.has(id)) {
  	instantiationService = globalInstantiationService;
  	} else if (editor) {
  	instantiationService = editor._instantiationService;
  	}
  	if (!instantiationService) {
  	return;
  	}
  	return instantiationService.invokeFunction(
  	monacoCommands.get(id)!.handler,
  	...args
  	);
  	},
  	isEnabled: () => {
  	const editor = codeEditorService.getActiveCodeEditor();
  	if (editorActions.has(id)) {
  	const action = editor && editor.getAction(id);
  	return !!action && action.isSupported();
  	}
  	if (editorRegistry.getEditorCommand(id)) {
  	return !!editor;
  	}
  	return true;
  	}
  	};
  	const label = editorActions.get(id);
  	this.commandRegistry.registerCommand({ id, label }, handler);
  	const coreCommand = MonacoCommands.COMMON_ACTIONS.get(id);
  	if (coreCommand) {
  	this.commandRegistry.registerHandler(coreCommand, handler);
  	}
  	}
  	}

• Adds integration tests for Find/Replace/Undo/Redo/Select All commands

How to test

• Find/Replace -> if the current editor then it should be used, if not then either suppressed or a widget should be used, i.e. the terminal
  • integration tests added to cover common cases
• Undo/Redo/Select All -> if the current editor then it should be used, otherwise delegate to DOM, rely on Monaco for implementation. Check input areas with and without editors in other views, git, SIW, terminal find, webview (gitlens welcome)
  • integration tests added to cover common cases
• Editor Actions like manipulation of selections
  • should be available in the command palette when the current editor is available and work properly
  • language features should be disable when they are not provided even for the current editor
  • test inline editors as debug console input and breakpoint expression editor
  • test embedded editors as peek references
• there should NOT be any exceptions in logs about missing service if the gitlens extension is installed
• integration tests should pass

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[kittaakos]

Executing Undo whilst the focus is an input; executes the command in the editor:

[kittaakos]

Monaco editor commands incorrectly execute with the outer editor context, instead of the embedded one. Steps to reproduce:

• Open an editor,
• Open an embedded editor,
• Set the focus in the inner editor,
• Execute Copy Line Up, the command will copy a line based on the selection in the outer editor instead of the inner one.

[akosyakov]

Executing Undo whilst the focus is an input; executes the command in the editor:

You did not do any changes in the input, only focused it? I think it is the same in VS Code.

[kittaakos]

How can I verify this change?

[akosyakov]

You should be able to call these commands via CommandService. I think we did it already by using undo/redo/select and find.

The issue with this code that it tries to override commands contributed by the monaco extension. In the good case it does nothing, just a warning in logs that such command already registered (what is happening on master i believe), in the worse it registers before monaco and causes infinity recursion.

[akosyakov]

It works in Code:

@kittaakos in VS Code git input is a Monaco editor, try with the search in workspace input

[kittaakos]

@kittaakos in VS Code git input is a Monaco editor, try with the search in workspace input

You're right. The behavior isn't nice in VS Code editor, but Theia works as VS Code. I had to use the SIW input instead of the SCM textArea monaco-editor.

[akosyakov]

Monaco editor commands incorrectly execute with the outer editor context, instead of the embedded one. Steps to reproduce:

You are right about getFocusedCodeEditor. It has to be checked as well before getActiveCodeEditor. Focused code editors include inline and embedded editors, active code editors include only last focused standalone or diff editors.

I've adjuster handlers and added js-docs to exposed getFocusedCodeEditor to make it clear.

[akosyakov]

@kittaakos I've answered/addressed all your comments.

Dirty editor tests should be good now too. Code was assuming that we always load editor model to open from the disk or already processed all disk changes. But tests revealed that there are cases when we did not receive all events yet but already trying to open an editor. I've added a guard that if a model looks invalid then sync with the disk before throwing an invalid model exception.

I've also updated How to test section to reflect inline and embedded editors.

[kittaakos]

@akosyakov, can you please rebase from the master HEAD? I want to verify #7523. Currently, it does not work.

[akosyakov]

@kittaakos done, trying it as well as soon as the local build is through

[kittaakos]

I have verified it once more with the electron example; it works as expected! Very slick fix 👍

[tavoda]

git pull (I'm on master)
yarn build
cd examples/browser
yarn start

Still same error. Using theia/plugins/vscodevim.vim-1.13.1.vsix with size 1952450. Do I have to do some 'clean' or using different vscode-vim plugin from other registry?

[tavoda]

Found something at startup for CTRL+F:
root WARN A command editor.action.startFindReplaceAction is already registered.
root WARN Collided keybinding is ignored; {"command":"passthrough","keybinding":"ctrl+f","context":"terminalActive"} collided with {"command":"terminal:find","keybinding":
"ctrlcmd+f","context":"terminalActive"}
root WARN Could not register keybinding:
{"command":"passthrough","keybinding":"ctrl+f","context":"terminalActive"}
Error: "ctrl+f" is in collision with something else [scope:0]
root WARN Collided keybinding is ignored; {"command":"passthrough","keybinding":"ctrl+k","context":"terminalActive"} collided with {"command":"terminal:clear","keybinding"
:"ctrlcmd+k","context":"terminalActive"}
root WARN Could not register keybinding:
{"command":"passthrough","keybinding":"ctrl+k","context":"terminalActive"}
Error: "ctrl+k" is in collision with something else [scope:0]

[akosyakov]

A command editor.action.startFindReplaceAction is already registered.

@tavoda sounds like it was old version. This PR removes double registration: #7539 (comment)

[tavoda]

Once more:
git pull
yarn run clean
yarn run build
cd examples/browser
yarn start -> ERROR?!?!?
yarn build
yarn start

EVERYTHING OK, CTRL + F is working 😎 thank you
Now I can try to replace VSCODE with THEIA for everyday work.

[tavoda]

Hmmm too soon, now basic movements like hjkl is not working, instead of movement it looks like in insert mode and <ESC> doesn't help. However CTRL+F is working 😢

[akosyakov]

@tavoda thanks for checking, please open an issue 🙏