From 842196b5b18ce340f6b1beaea9e9a7ddbe2fd96f Mon Sep 17 00:00:00 2001 From: Anton Kosyakov Date: Fri, 17 Aug 2018 07:54:43 +0000 Subject: [PATCH] fix #2499: fixed Tree construction Signed-off-by: Anton Kosyakov --- packages/core/src/browser/tree/tree.spec.ts | 149 ++++++++++++++++++++ packages/core/src/browser/tree/tree.ts | 74 ++++++---- packages/markers/src/browser/marker-tree.ts | 4 +- 3 files changed, 200 insertions(+), 27 deletions(-) create mode 100644 packages/core/src/browser/tree/tree.spec.ts diff --git a/packages/core/src/browser/tree/tree.spec.ts b/packages/core/src/browser/tree/tree.spec.ts new file mode 100644 index 0000000000000..300b707fc6414 --- /dev/null +++ b/packages/core/src/browser/tree/tree.spec.ts @@ -0,0 +1,149 @@ +/******************************************************************************** + * Copyright (C) 2018 TypeFox 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 * as assert from 'assert'; +import { TreeNode, CompositeTreeNode } from './tree'; + +describe('Tree', () => { + + it('addChildren', () => { + assertTreeNode(`{ + "id": "parent", + "name": "parent", + "children": [ + { + "id": "foo", + "name": "foo", + "parent": "parent", + "nextSibling": "bar" + }, + { + "id": "bar", + "name": "bar", + "parent": "parent", + "previousSibling": "foo", + "nextSibling": "baz" + }, + { + "id": "baz", + "name": "baz", + "parent": "parent", + "previousSibling": "bar" + } + ] +}`, getNode()); + }); + + it('removeChild - first', () => { + const node = getNode(); + CompositeTreeNode.removeChild(node, node.children[0]); + assertTreeNode(`{ + "id": "parent", + "name": "parent", + "children": [ + { + "id": "bar", + "name": "bar", + "parent": "parent", + "nextSibling": "baz" + }, + { + "id": "baz", + "name": "baz", + "parent": "parent", + "previousSibling": "bar" + } + ] +}`, node); + }); + + it('removeChild - second', () => { + const node = getNode(); + CompositeTreeNode.removeChild(node, node.children[1]); + assertTreeNode(`{ + "id": "parent", + "name": "parent", + "children": [ + { + "id": "foo", + "name": "foo", + "parent": "parent", + "nextSibling": "baz" + }, + { + "id": "baz", + "name": "baz", + "parent": "parent", + "previousSibling": "foo" + } + ] +}`, node); + }); + + it('removeChild - thrid', () => { + const node = getNode(); + CompositeTreeNode.removeChild(node, node.children[2]); + assertTreeNode(`{ + "id": "parent", + "name": "parent", + "children": [ + { + "id": "foo", + "name": "foo", + "parent": "parent", + "nextSibling": "bar" + }, + { + "id": "bar", + "name": "bar", + "parent": "parent", + "previousSibling": "foo" + } + ] +}`, node); + }); + + function getNode(): CompositeTreeNode { + return CompositeTreeNode.addChildren({ + id: 'parent', + name: 'parent', + children: [], + parent: undefined + }, [{ + id: 'foo', + name: 'foo', + parent: undefined + }, { + id: 'bar', + name: 'bar', + parent: undefined + }, { + id: 'baz', + name: 'baz', + parent: undefined + }]); + } + + function assertTreeNode(expectation: string, node: TreeNode): void { + assert.deepEqual(expectation, JSON.stringify(node, (key: keyof CompositeTreeNode, value: any) => { + if (key === 'parent' || key === 'previousSibling' || key === 'nextSibling') { + return value && value.id; + } + return value; + }, 2)); + } + +}); diff --git a/packages/core/src/browser/tree/tree.ts b/packages/core/src/browser/tree/tree.ts index f03e1bc4c86db..57be8d7b59e35 100644 --- a/packages/core/src/browser/tree/tree.ts +++ b/packages/core/src/browser/tree/tree.ts @@ -144,6 +144,54 @@ export namespace CompositeTreeNode { } return parent.children.findIndex(child => TreeNode.equals(node, child)); } + + export function addChildren(parent: CompositeTreeNode, children: TreeNode[]): CompositeTreeNode { + for (const child of children) { + addChild(parent, child); + } + return parent; + } + + export function addChild(parent: CompositeTreeNode, child: TreeNode): CompositeTreeNode { + const children = parent.children as TreeNode[]; + const index = children.findIndex(value => value.id === child.id); + if (index !== -1) { + children.splice(index, 1, child); + setParent(child, index, parent); + } else { + children.push(child); + setParent(child, parent.children.length - 1, parent); + } + return parent; + } + + export function removeChild(parent: CompositeTreeNode, child: TreeNode): void { + const children = parent.children as TreeNode[]; + const index = children.findIndex(value => value.id === child.id); + if (index === -1) { + return; + } + children.splice(index, 1); + const { previousSibling, nextSibling } = child; + if (previousSibling) { + Object.assign(previousSibling, { nextSibling }); + } + if (nextSibling) { + Object.assign(nextSibling, { previousSibling }); + } + } + + export function setParent(child: TreeNode, index: number, parent: CompositeTreeNode): void { + const previousSibling = parent.children[index - 1]; + const nextSibling = parent.children[index + 1]; + Object.assign(child, { parent, previousSibling, nextSibling }); + if (previousSibling) { + Object.assign(previousSibling, { nextSibling: child }); + } + if (nextSibling) { + Object.assign(nextSibling, { previousSibling: child }); + } + } } /** @@ -246,34 +294,10 @@ export class TreeImpl implements Tree { if (CompositeTreeNode.is(node)) { const { children } = node; children.forEach((child, index) => { - this.setParent(child, index, node); + CompositeTreeNode.setParent(child, index, node); this.addNode(child); }); } } - protected setParent(child: TreeNode, index: number, parent: CompositeTreeNode): void { - const previousSibling = parent.children[index - 1]; - const nextSibling = parent.children[index + 1]; - Object.assign(child, { parent, previousSibling, nextSibling }); - } - - protected addChild(parent: CompositeTreeNode, child: TreeNode): void { - const index = parent.children.findIndex(value => value.id === child.id); - if (index !== -1) { - (parent.children as TreeNode[]).splice(index, 1, child); - this.setParent(child, index, parent); - } else { - (parent.children as TreeNode[]).push(child); - this.setParent(child, parent.children.length - 1, parent); - } - } - - protected removeChild(parent: CompositeTreeNode, child: TreeNode): void { - const index = parent.children.findIndex(value => value.id === child.id); - if (index !== -1) { - (parent.children as TreeNode[]).splice(index, 1); - } - } - } diff --git a/packages/markers/src/browser/marker-tree.ts b/packages/markers/src/browser/marker-tree.ts index 1948ab209b6f9..244b5d843be46 100644 --- a/packages/markers/src/browser/marker-tree.ts +++ b/packages/markers/src/browser/marker-tree.ts @@ -55,14 +55,14 @@ export abstract class MarkerTree extends TreeImpl { const markers = this.markerManager.findMarkers({ uri }); if (markers.length <= 0) { if (MarkerInfoNode.is(existing)) { - this.removeChild(existing.parent, existing); + CompositeTreeNode.removeChild(existing.parent, existing); this.removeNode(existing); this.fireChanged(); } return; } const node = MarkerInfoNode.is(existing) ? existing : await this.createMarkerInfo(id, uri); - this.addChild(node.parent, node); + CompositeTreeNode.addChild(node.parent, node); const children = this.getMarkerNodes(node, markers); node.numberOfMarkers = markers.length; this.setChildren(node, children);