Fix SCM Inline commands

SCM inline commands were executed before adapting their arguments to its 'rpc' format, causing an infinite loop while trying to serialize argument objects before sending them to the 'rpc' channel. The adapter's exist and were originally used by context menus. This update uses the 'MenuCommandExecutor' to channel the execution of SCM inline commands through the existing registered adapters. It also replaces the use of 'CommandRegistry' by 'MenuCommandExecutor' Signed-off-by: Alvaro Sanchez-Leon <alvaro.sanchez-leon@ericsson.com>
eclipse-theia · Mar 17, 2023 · de84d3a · de84d3a
1 parent 16be9ac
commit de84d3a
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,10 @@
 <a name="breaking_changes_1.36.0">[Breaking Changes:](#breaking_changes_1.36.0)</a>
 
 - [plugin] renamed `TreeViewExtImpl#toTreeItem()` to `TreeViewExtImpl#toTreeElement()`
+- [scm] Fixing 'scm' inline commands, introduces the following breaking changes: [#12295](https://github.com/eclipse-theia/theia/pull/12295)
+    - Interface ScmInlineAction removes 'commands: CommandRegistry'
+    - Interface ScmInlineActions removes 'commands: CommandRegistry'
+    - Interface ScmTreeWidget.Props removes 'commands: CommandRegistry'
 
 ## v1.35.0 - 02/23/2023
 

diff --git a/packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts b/packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts
@@ -98,7 +98,7 @@ export class PluginMenuCommandAdapter implements MenuCommandAdapter {
             ['scm/resourceFolder/context', toScmArgs],
             ['scm/resourceGroup/context', toScmArgs],
             ['scm/resourceState/context', toScmArgs],
-            ['scm/title', () => this.toScmArg(this.scmService.selectedRepository)],
+            ['scm/title', () => [this.toScmArg(this.scmService.selectedRepository)]],
             ['timeline/item/context', (...args) => this.toTimelineArgs(...args)],
             ['view/item/context', (...args) => this.toTreeArgs(...args)],
             ['view/title', noArgs],

diff --git a/packages/scm/src/browser/scm-tree-widget.tsx b/packages/scm/src/browser/scm-tree-widget.tsx
@@ -23,9 +23,8 @@ import { isOSX } from '@theia/core/lib/common/os';
 import { DisposableCollection, Disposable } from '@theia/core/lib/common/disposable';
 import { TreeWidget, TreeNode, SelectableTreeNode, TreeModel, TreeProps, NodeProps, TREE_NODE_SEGMENT_CLASS, TREE_NODE_SEGMENT_GROW_CLASS } from '@theia/core/lib/browser/tree';
 import { ScmTreeModel, ScmFileChangeRootNode, ScmFileChangeGroupNode, ScmFileChangeFolderNode, ScmFileChangeNode } from './scm-tree-model';
-import { MenuModelRegistry, ActionMenuNode, CompoundMenuNode, MenuPath } from '@theia/core/lib/common/menu';
+import { MenuCommandExecutor, MenuModelRegistry, ActionMenuNode, CompoundMenuNode, MenuPath } from '@theia/core/lib/common/menu';
 import { ScmResource } from './scm-provider';
-import { CommandRegistry } from '@theia/core/lib/common/command';
 import { ContextMenuRenderer, LabelProvider, CorePreferences, DiffUris, ACTION_ITEM } from '@theia/core/lib/browser';
 import { ScmContextKeyService } from './scm-context-key-service';
 import { EditorWidget, EditorManager, DiffNavigatorProvider } from '@theia/editor/lib/browser';
@@ -48,8 +47,8 @@ export class ScmTreeWidget extends TreeWidget {
     static RESOURCE_CONTEXT_MENU = ['RESOURCE_CONTEXT_MENU'];
     static RESOURCE_INLINE_MENU = ['RESOURCE_CONTEXT_MENU', 'inline'];
 
+    @inject(MenuCommandExecutor) protected readonly menuCommandExecutor: MenuCommandExecutor;
     @inject(MenuModelRegistry) protected readonly menus: MenuModelRegistry;
-    @inject(CommandRegistry) protected readonly commands: CommandRegistry;
     @inject(ScmContextKeyService) protected readonly contextKeys: ScmContextKeyService;
     @inject(EditorManager) protected readonly editorManager: EditorManager;
     @inject(DiffNavigatorProvider) protected readonly diffNavigatorProvider: DiffNavigatorProvider;
@@ -102,8 +101,8 @@ export class ScmTreeWidget extends TreeWidget {
                 model={this.model}
                 treeNode={node}
                 renderExpansionToggle={() => this.renderExpansionToggle(node, props)}
+                commandExecutor={this.menuCommandExecutor}
                 contextMenuRenderer={this.contextMenuRenderer}
-                commands={this.commands}
                 menus={this.menus}
                 contextKeys={this.contextKeys}
                 labelProvider={this.labelProvider}
@@ -121,8 +120,8 @@ export class ScmTreeWidget extends TreeWidget {
                 treeNode={node}
                 sourceUri={node.sourceUri}
                 renderExpansionToggle={() => this.renderExpansionToggle(node, props)}
+                commandExecutor={this.menuCommandExecutor}
                 contextMenuRenderer={this.contextMenuRenderer}
-                commands={this.commands}
                 menus={this.menus}
                 contextKeys={this.contextKeys}
                 labelProvider={this.labelProvider}
@@ -142,7 +141,7 @@ export class ScmTreeWidget extends TreeWidget {
                 model={this.model}
                 treeNode={node}
                 contextMenuRenderer={this.contextMenuRenderer}
-                commands={this.commands}
+                commandExecutor={this.menuCommandExecutor}
                 menus={this.menus}
                 contextKeys={this.contextKeys}
                 labelProvider={this.labelProvider}
@@ -455,7 +454,6 @@ export namespace ScmTreeWidget {
     export interface Props {
         treeNode: TreeNode;
         model: ScmTreeModel;
-        commands: CommandRegistry;
         menus: MenuModelRegistry;
         contextKeys: ScmContextKeyService;
         labelProvider: LabelProvider;
@@ -518,7 +516,8 @@ export abstract class ScmElement<P extends ScmElement.Props = ScmElement.Props>
 }
 export namespace ScmElement {
     export interface Props extends ScmTreeWidget.Props {
-        renderExpansionToggle: () => React.ReactNode
+        renderExpansionToggle: () => React.ReactNode;
+        commandExecutor: MenuCommandExecutor;
     }
     export interface State {
         hover: boolean
@@ -529,7 +528,7 @@ export class ScmResourceComponent extends ScmElement<ScmResourceComponent.Props>
 
     override render(): JSX.Element | undefined {
         const { hover } = this.state;
-        const { model, treeNode, colors, parentPath, sourceUri, decoration, labelProvider, commands, menus, contextKeys, caption } = this.props;
+        const { model, treeNode, colors, parentPath, sourceUri, decoration, labelProvider, commandExecutor, menus, contextKeys, caption } = this.props;
         const resourceUri = new URI(sourceUri);
 
         const icon = labelProvider.getIcon(resourceUri);
@@ -561,8 +560,9 @@ export class ScmResourceComponent extends ScmElement<ScmResourceComponent.Props>
             <ScmInlineActions {...{
                 hover,
                 menu: menus.getMenu(ScmTreeWidget.RESOURCE_INLINE_MENU),
+                menuPath: ScmTreeWidget.RESOURCE_INLINE_MENU,
+                commandExecutor,
                 args: this.contextMenuArgs,
-                commands,
                 contextKeys,
                 model,
                 treeNode
@@ -644,7 +644,7 @@ export class ScmResourceGroupElement extends ScmElement<ScmResourceGroupComponen
 
     override render(): JSX.Element {
         const { hover } = this.state;
-        const { model, treeNode, menus, commands, contextKeys, caption } = this.props;
+        const { model, treeNode, menus, commandExecutor, contextKeys, caption } = this.props;
         return <div className={`theia-header scm-theia-header ${TREE_NODE_SEGMENT_GROW_CLASS}`}
             onContextMenu={this.renderContextMenu}
             onMouseEnter={this.showHover}
@@ -656,7 +656,8 @@ export class ScmResourceGroupElement extends ScmElement<ScmResourceGroupComponen
                 hover,
                 args: this.contextMenuArgs,
                 menu: menus.getMenu(ScmTreeWidget.RESOURCE_GROUP_INLINE_MENU),
-                commands,
+                menuPath: ScmTreeWidget.RESOURCE_GROUP_INLINE_MENU,
+                commandExecutor,
                 contextKeys,
                 model,
                 treeNode
@@ -694,7 +695,7 @@ export class ScmResourceFolderElement extends ScmElement<ScmResourceFolderElemen
 
     override render(): JSX.Element {
         const { hover } = this.state;
-        const { model, treeNode, sourceUri, labelProvider, commands, menus, contextKeys, caption } = this.props;
+        const { model, treeNode, sourceUri, labelProvider, commandExecutor, menus, contextKeys, caption } = this.props;
         const sourceFileStat = FileStat.dir(sourceUri);
         const icon = labelProvider.getIcon(sourceFileStat);
         const title = new URI(sourceUri).path.fsPath();
@@ -715,8 +716,9 @@ export class ScmResourceFolderElement extends ScmElement<ScmResourceFolderElemen
             <ScmInlineActions {...{
                 hover,
                 menu: menus.getMenu(ScmTreeWidget.RESOURCE_FOLDER_INLINE_MENU),
+                menuPath: ScmTreeWidget.RESOURCE_FOLDER_INLINE_MENU,
+                commandExecutor,
                 args: this.contextMenuArgs,
-                commands,
                 contextKeys,
                 model,
                 treeNode
@@ -750,11 +752,12 @@ export namespace ScmResourceFolderElement {
 
 export class ScmInlineActions extends React.Component<ScmInlineActions.Props> {
     override render(): React.ReactNode {
-        const { hover, menu, args, commands, model, treeNode, contextKeys, children } = this.props;
+        const { hover, menu, menuPath, args, commandExecutor, model, treeNode, contextKeys, children } = this.props;
         return <div className='theia-scm-inline-actions-container'>
             <div className='theia-scm-inline-actions'>
                 {hover && menu.children
-                    .map((node, index) => node instanceof ActionMenuNode && <ScmInlineAction key={index} {...{ node, args, commands, model, treeNode, contextKeys }} />)}
+                    .map((node, index) => node instanceof ActionMenuNode &&
+                        <ScmInlineAction key={index} {...{ node, menuPath, args, commandExecutor, model, treeNode, contextKeys }} />)}
             </div>
             {children}
         </div>;
@@ -764,7 +767,8 @@ export namespace ScmInlineActions {
     export interface Props {
         hover: boolean;
         menu: CompoundMenuNode;
-        commands: CommandRegistry;
+        menuPath: MenuPath;
+        commandExecutor: MenuCommandExecutor;
         model: ScmTreeModel;
         treeNode: TreeNode;
         contextKeys: ScmContextKeyService;
@@ -775,14 +779,14 @@ export namespace ScmInlineActions {
 
 export class ScmInlineAction extends React.Component<ScmInlineAction.Props> {
     override render(): React.ReactNode {
-        const { node, model, treeNode, args, commands, contextKeys } = this.props;
+        const { node, model, treeNode, args, commandExecutor, menuPath, contextKeys } = this.props;
 
         let isActive: boolean = false;
         model.execInNodeContext(treeNode, () => {
             isActive = contextKeys.match(node.when);
         });
 
-        if (!commands.isVisible(node.command, ...args) || !isActive) {
+        if (!commandExecutor.isVisible(menuPath, node.command, ...args) || !isActive) {
             return false;
         }
         return <div className='theia-scm-inline-action'>
@@ -793,14 +797,15 @@ export class ScmInlineAction extends React.Component<ScmInlineAction.Props> {
     protected execute = (event: React.MouseEvent) => {
         event.stopPropagation();
 
-        const { commands, node, args } = this.props;
-        commands.executeCommand(node.command, ...args);
+        const { commandExecutor, menuPath, node, args } = this.props;
+        commandExecutor.executeCommand([menuPath[0]], node.command, ...args);
     };
 }
 export namespace ScmInlineAction {
     export interface Props {
         node: ActionMenuNode;
-        commands: CommandRegistry;
+        commandExecutor: MenuCommandExecutor;
+        menuPath: MenuPath;
         model: ScmTreeModel;
         treeNode: TreeNode;
         contextKeys: ScmContextKeyService;