add dependency treeview #647

ShihanMeng618 · 2021-06-28T03:08:03Z

To implement dependency tree requested in #161,
and the dependencies can be expanded and collapsed.

it also includes information about duplicates and conflicts:

Eskibear · 2021-06-29T08:51:15Z

/azp run

azure-pipelines · 2021-06-29T08:51:22Z

Azure Pipelines successfully started running 1 pipeline(s).

Eskibear

Overall it's a working, but the implemetation can be refined.

What I think of the workflow is:

run some maven commands in background to fetch a raw data of dependencies in plaintext format.
parse plaintext, you get a dependency tree.
for each tree node, implement ITreeItem to tell vscode how to create a corresponding TreeItem for each dependency.

BTW, CI is failing, please also fix it.

src/utils/mavenUtils.ts

Eskibear · 2021-06-29T09:09:31Z

src/utils/mavenUtils.ts

@@ -83,7 +91,7 @@ async function executeInBackground(mvnArgs: string, pomfile?: string): Promise<a
                if (code === 0) {
                    resolve(code);
                } else {
-                    reject(new Error(`Background process terminated with code ${code}.`));
+                    reject(new Error(`Background process terminated with code ${code} and the failed command can be found in output channel.`));


extra information/guidance should be provided outside.

Let's leave it to another PR, which improves overall UX of error handling

Eskibear · 2021-06-29T09:11:28Z

src/explorer/model/Dependencies.ts

+const DUPLICATE_VALUE: string = "omitted for duplicate";
+const CONFLICT_VALUE: string = "omitted for conflict";
+
+export class Dependencies implements ITreeItem {


Use Dependency or MavenDependency instead of Dependencies

What I imagine:

basic properties of a Dependency should at least include groupId, artifactId, version, scope, which are parsed from plaintext from your implementation.

in getTreeItem, you contruct an TreeItem using above props.

Eskibear · 2021-06-29T09:12:34Z

src/explorer/model/DependenciesMenu.ts

+        treeContent = treeContent.replace(/\|/g, " ");
+        treeContent = treeContent.replace(/\\/g, "+");
+        treeContent = treeContent.replace(/\n/g, "\r\n");
+        return Promise.resolve(this.getDepsInString(treeContent));


getDepsInString => parseDependencies

Eskibear · 2021-06-29T09:24:45Z

src/explorer/model/Dependencies.ts

+    public dependency: string;
+    public pomPath: string;
+    private curDep: string;
+    private separator: string;


what's it for?

src/explorer/model/Dependencies.ts

Eskibear · 2021-06-29T09:32:40Z

src/explorer/model/Dependencies.ts

+    }
+
+    public getTreeItem(): vscode.TreeItem | Thenable<vscode.TreeItem> {
+        this.curDep = this.dependency.split(this.separator, 1)[0];


parsing work should be extracted into another utility.

during parsing with some complicated regular expression, it's better to add comments about i) a sample input and ii) the matched parts.

Eskibear · 2021-06-29T09:50:14Z

npm run tslint locally before you commit

…code.TreeItem

…dy618/vscode-maven into resolve-dependency-conflict

Eskibear · 2021-07-05T02:01:16Z

src/utils/mavenUtils.ts

+    const outputDirectory: string = path.dirname(pomPath);
+    const outputFileName: string = "dependency-graph";
+    await executeInBackground(`depgraph:graph -DgraphFormat=text -DshowDuplicates -DshowConflicts -DshowVersions -DshowGroupIds -DoutputDirectory="${outputDirectory}" -DoutputFileName="${outputFileName}"`, pomPath);
+    return await readFileIfExists(`${outputDirectory}\\${outputFileName}.txt`);


\\ might not work with Unix-like systems

use path.join

Eskibear

Dependency itself can be tree-like, I think we don't have to create an extra class TreeNode.
Does below structure work?

Dependency {
parent: Dependency | undefined
addChild(value: string | Dependency)
// other props
....
}

Eskibear · 2021-07-05T02:06:58Z

src/explorer/model/DependenciesMenu.ts

+    }
+}
+
+export function parseDependency(parentNode: TreeNode): Dependency[] {


Don't export it. If you want to reuse this function, place it into utilities.

Eskibear · 2021-07-05T02:08:16Z

src/explorer/model/TreeNode.ts

+        this.value = value;
+    }
+
+    public addChildValue(value: string): void {


addChild(value: string) is simpler and carries enough information.

Eskibear · 2021-07-05T02:09:54Z

src/explorer/model/TreeNode.ts

+        this.children.push(child);
+    }
+
+    public addChildNode(node: TreeNode): void {


addChild(node: TreeNode) is enough.
See https://www.typescriptlang.org/docs/handbook/2/functions.html#function-overloads for overloads

Eskibear · 2021-07-05T02:11:51Z

src/explorer/model/Dependency.ts

+// Licensed under the MIT license.
+
+import * as vscode from "vscode";
+import { parseDependency } from "./DependenciesMenu";


it should be independent with DependenciesMenu

Eskibear · 2021-07-05T02:17:14Z

src/handlers/parseRawDependencyDataHandler.ts

+    return rootNode;
+}
+
+function parseTreeNodes(parentNode: TreeNode, treecontent: string, separator: string, indent: string, starter: string, eol: string): void {


Instead of passing a parent node and modifying it, parseTreeNodes should directly parse a raw string and return an array of TreeNode.

Eskibear · 2021-07-05T02:20:55Z

src/handlers/parseRawDependencyDataHandler.ts

+    const indent: string = "   "; // three spaces
+    const separator: string = "\r\n";
+    const starter: string = "+- ";
+    const rootNode: TreeNode = new TreeNode("Dependencies");


The node new TreeNode("Dependencies") is dummy.
Can you simply return an array of top level dependencies here?

dependencies treeview

a3fc2e5

ShihanMeng618 marked this pull request as draft June 28, 2021 03:15

ShihanMeng618 marked this pull request as ready for review June 28, 2021 03:15

ShihanMeng618 and others added 3 commits June 28, 2021 11:35

fix CI errors

f96dfa3

fix other ci errors

b90073f

Merge branch 'main' into resolve-dependency-conflict

d9a899b

Eskibear requested changes Jun 29, 2021

View reviewed changes

ShihanMeng618 added 3 commits July 4, 2021 10:45

refine the implementation by adding TreeNode between plaintext and vs…

6d5a042

…code.TreeItem

Merge branch 'resolve-dependency-conflict' of https://github.com/Melo…

fb06370

…dy618/vscode-maven into resolve-dependency-conflict

fix CI, delete unread variables in class Dependency

9def58e

Eskibear reviewed Jul 5, 2021

View reviewed changes

Eskibear requested changes Jul 5, 2021

View reviewed changes

ShihanMeng618 added 2 commits July 6, 2021 13:51

Dependency entends TreeNode

88fcc79

delete unnecessary error message

e0177bc

Eskibear approved these changes Jul 6, 2021

View reviewed changes

Eskibear merged commit 47b2964 into microsoft:main Jul 6, 2021

Eskibear mentioned this pull request Jul 19, 2021

"Show Dependencies" does not resolve sibling module #650

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add dependency treeview #647

add dependency treeview #647

ShihanMeng618 commented Jun 28, 2021 •

edited

Loading

Eskibear commented Jun 29, 2021

azure-pipelines bot commented Jun 29, 2021

Eskibear left a comment

Eskibear Jun 29, 2021

Eskibear Jun 29, 2021

Eskibear Jun 29, 2021

Eskibear Jun 29, 2021

Eskibear Jun 29, 2021

Eskibear Jun 29, 2021

Eskibear Jun 29, 2021

Eskibear commented Jun 29, 2021

Eskibear Jul 5, 2021

Eskibear Jul 5, 2021

Eskibear left a comment

Eskibear Jul 5, 2021

Eskibear Jul 5, 2021

Eskibear Jul 5, 2021

Eskibear Jul 5, 2021

Eskibear Jul 5, 2021

Eskibear Jul 5, 2021

add dependency treeview #647

add dependency treeview #647

Conversation

ShihanMeng618 commented Jun 28, 2021 • edited Loading

Eskibear commented Jun 29, 2021

azure-pipelines bot commented Jun 29, 2021

Eskibear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Eskibear commented Jun 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Eskibear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShihanMeng618 commented Jun 28, 2021 •

edited

Loading