Debug variable nodes kept expanded while stepping (#10338)

* Debug variable nodes kept expanded while stepping Signed-off-by: Federico Bozzini <[email protected]> * Expansion cache refreshed on source tree refresh Signed-off-by: Federico Bozzini <[email protected]> * Local variables top level always expanded Signed-off-by: Federico Bozzini <[email protected]> * Refactoring Signed-off-by: Federico Bozzini <[email protected]> * Added comment Signed-off-by: Federico Bozzini <[email protected]> * Improved comments Signed-off-by: Federico Bozzini <[email protected]> * Extracted constant for annotating tree nodes Signed-off-by: Federico Bozzini <[email protected]> --------- Signed-off-by: Federico Bozzini <[email protected]>
eclipse-theia · Mar 15, 2023 · db48de3 · db48de3
1 parent c7ca0d5
commit db48de3
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 4 deletions.
diff --git a/packages/core/src/browser/source-tree/source-tree.ts b/packages/core/src/browser/source-tree/source-tree.ts
@@ -21,6 +21,9 @@ import { MaybePromise } from '../../common/types';
 import { TreeImpl, CompositeTreeNode, TreeNode, SelectableTreeNode, ExpandableTreeNode } from '../tree';
 import { TreeElement, CompositeTreeElement, TreeSource } from './tree-source';
 
+export const SOURCE_NODE_ID_PREFIX = '__source__';
+export const SOURCE_NODE_ID_SEPARATOR = ':';
+
 @injectable()
 export class SourceTree extends TreeImpl {
 
@@ -44,7 +47,7 @@ export class SourceTree extends TreeImpl {
     }
 
     protected toNode(element: TreeElement, index: number, parent: TreeElementNodeParent): TreeElementNode {
-        const id = element.id ? String(element.id) : (parent.id + ':' + index);
+        const id = element.id ? String(element.id) : (parent.id + SOURCE_NODE_ID_SEPARATOR + index);
         const name = id;
         const existing = this.getNode(id);
         const updated = existing && <TreeElementNode>Object.assign(existing, { element, parent });
@@ -132,7 +135,7 @@ export namespace TreeSourceNode {
         if (!source) {
             return source;
         }
-        const id = source.id || '__source__';
+        const id = source.id || SOURCE_NODE_ID_PREFIX;
         return {
             id,
             name: id,

diff --git a/packages/debug/src/browser/debug-variables-source-tree.ts b/packages/debug/src/browser/debug-variables-source-tree.ts
@@ -0,0 +1,92 @@
+/********************************************************************************
+ * Copyright (C) 2021 ARM and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { inject, injectable, postConstruct } from '@theia/core/shared/inversify';
+import { ExpandableTreeNode, TreeNode } from '@theia/core/lib/browser';
+import { SourceTree, TreeElement, TreeElementNode, TreeElementNodeParent, SOURCE_NODE_ID_PREFIX, SOURCE_NODE_ID_SEPARATOR } from '@theia/core/lib/browser/source-tree';
+import { Disposable } from '@theia/core';
+import { DebugScope, DebugVariable } from './console/debug-console-items';
+import { DebugSessionManager } from './debug-session-manager';
+
+@injectable()
+export class DebugVariablesSourceTree extends SourceTree {
+
+    @inject(DebugSessionManager)
+    protected readonly debugSessionsManager: DebugSessionManager;
+
+    @postConstruct()
+    protected init(): void {
+        this.toDispose.push(Disposable.create(() => this.expandedElements.clear()));
+        this.toDispose.push(this.debugSessionsManager.onDidStartDebugSession(() => {
+            this.firstNodeExpanded = true;
+            this.expandedElements.clear();
+        }));
+    }
+
+    protected readonly expandedElements = new Set<string>();
+    protected firstNodeExpanded = true;
+    /**
+     * Id of the first node of the tree
+     */
+    protected readonly firstNodeId = `${SOURCE_NODE_ID_PREFIX}${SOURCE_NODE_ID_SEPARATOR}0`;
+
+    async resolveChildren(parent: TreeElementNodeParent): Promise<TreeNode[]> {
+        const nodes = await super.resolveChildren(parent);
+        nodes.forEach(node => {
+            if (!ExpandableTreeNode.is(node)
+            || !TreeElementNode.is(node)
+            || !this.isDebugScopeOrVariable(node.element)) {
+                return;
+            }
+            const elementId = this.getNodeElementId(node);
+            if (this.expandedElements.has(elementId)
+                || (this.firstNodeExpanded && node.id === this.firstNodeId)) {
+                node.expanded = true;
+            }
+        });
+        return nodes;
+    }
+
+    handleExpansion(node: Readonly<ExpandableTreeNode>): void {
+        if (!TreeElementNode.is(node) || !this.isDebugScopeOrVariable(node.element)) {
+            return;
+        }
+        const id = this.getNodeElementId(node);
+        if (node.expanded) {
+            this.expandedElements.add(id);
+        } else {
+            this.expandedElements.delete(id);
+            if (node.id === this.firstNodeId) {
+                this.firstNodeExpanded = false;
+            }
+        }
+    }
+    protected getNodeElementId(node: Readonly<TreeElementNode>): string {
+        /* node.id is a positional identifier (EG: __source__:0:1 identifies the second child of the first child of the root)
+         * so if used directly to memorize the expanded node it would expand tree nodes that should be collapsed.
+         * Using a combination of the node id and the node element name greately reduces the risks of collisions
+         */
+        if (this.isDebugScopeOrVariable(node.element)) {
+            return node.id + '-' + node.element.name;
+        }
+        return node.id;
+    }
+
+    protected isDebugScopeOrVariable(element: TreeElement): element is (DebugScope | DebugVariable) {
+        return element instanceof DebugScope || element instanceof DebugVariable;
+    }
+
+}
diff --git a/packages/debug/src/browser/debug-variables-tree-model.ts b/packages/debug/src/browser/debug-variables-tree-model.ts
@@ -0,0 +1,31 @@
+/********************************************************************************
+ * Copyright (C) 2021 ARM and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { inject } from '@theia/core/shared/inversify';
+import { ExpandableTreeNode, TreeModelImpl } from '@theia/core/lib/browser';
+import { DebugVariablesSourceTree } from './debug-variables-source-tree';
+
+export class DebugVariablesTreeModel extends TreeModelImpl {
+
+    @inject(DebugVariablesSourceTree) protected readonly tree: DebugVariablesSourceTree;
+
+    handleExpansion(node: Readonly<ExpandableTreeNode>): void {
+        super.handleExpansion(node);
+        if (this.tree.handleExpansion) {
+            this.tree.handleExpansion(node);
+        }
+    }
+}
diff --git a/packages/debug/src/browser/view/debug-variables-source.ts b/packages/debug/src/browser/view/debug-variables-source.ts
@@ -18,7 +18,6 @@ import { injectable, inject, postConstruct } from '@theia/core/shared/inversify'
 import { TreeSource } from '@theia/core/lib/browser/source-tree';
 import { DebugScope } from '../console/debug-console-items';
 import { DebugViewModel } from './debug-view-model';
-import debounce = require('p-debounce');
 
 @injectable()
 export class DebugVariablesSource extends TreeSource {
@@ -32,7 +31,7 @@ export class DebugVariablesSource extends TreeSource {
         this.toDispose.push(this.model.onDidChange(() => this.refresh()));
     }
 
-    protected readonly refresh = debounce(() => this.fireDidChange(), 400);
+    protected readonly refresh = () => this.fireDidChange();
 
     async getElements(): Promise<IterableIterator<DebugScope>> {
         const { currentSession } = this.model;

diff --git a/packages/debug/src/browser/view/debug-variables-widget.ts b/packages/debug/src/browser/view/debug-variables-widget.ts
@@ -20,6 +20,9 @@ import { SourceTreeWidget } from '@theia/core/lib/browser/source-tree';
 import { DebugVariablesSource } from './debug-variables-source';
 import { DebugViewModel } from './debug-view-model';
 import { nls } from '@theia/core/lib/common/nls';
+import { DebugVariablesSourceTree } from '../debug-variables-source-tree';
+import { Tree, TreeModel } from '@theia/core/lib/browser';
+import { DebugVariablesTreeModel } from '../debug-variables-tree-model';
 
 @injectable()
 export class DebugVariablesWidget extends SourceTreeWidget {
@@ -37,6 +40,11 @@ export class DebugVariablesWidget extends SourceTreeWidget {
         child.bind(DebugVariablesSource).toSelf();
         child.unbind(SourceTreeWidget);
         child.bind(DebugVariablesWidget).toSelf();
+        child.bind(DebugVariablesSourceTree).toSelf();
+        child.rebind(Tree).toService(DebugVariablesSourceTree);
+        child.bind(DebugVariablesTreeModel).toSelf();
+        child.rebind(TreeModel).toService(DebugVariablesTreeModel);
+
         return child;
     }
     static createWidget(parent: interfaces.Container): DebugVariablesWidget {