Skip to content

Commit

Permalink
Fix SCM Inline commands
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@alvsan09
alvsan09 committed Mar 17, 2023
1 parent 16be9ac commit de84d3a
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 23 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
49 changes: 27 additions & 22 deletions packages/scm/src/browser/scm-tree-widget.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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}
Expand All @@ -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}
Expand All @@ -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}
Expand Down Expand Up @@ -455,7 +454,6 @@ export namespace ScmTreeWidget {
export interface Props {
treeNode: TreeNode;
model: ScmTreeModel;
commands: CommandRegistry;
menus: MenuModelRegistry;
contextKeys: ScmContextKeyService;
labelProvider: LabelProvider;
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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>;
Expand All @@ -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;
Expand All @@ -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'>
Expand All @@ -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;
Expand Down

0 comments on commit de84d3a

Please sign in to comment.