Enhanced Tree view #94

Vigilans · 2019-01-23T05:09:50Z

Introduction

This PR restructures the left side panel's tree view to support following functions:

View problems by difficulty, tag or company Support label search, for example: array, map, greedy algorithms... #67
View favorite problems
Option to hide accepted questions (won't be hidden in Favorite view)

Details

Company and tag filtering is achieved through the leetcode-cli company plugin.
Problems with no tag or company knowledge are labeled as Unknown.
Solved problems can be hidden through leetcode.hideSolved setting, which defaults to false.

Demonstration

Difficulty, tag, company or favorite:

Problems by tag:

Problems by company:

Favorite problems:

(from direct import of leetcode-cli company plugin)

Add unknown label to companies and tags

jdneo · 2019-01-23T05:16:50Z

@Vigilans Thank you for contribution. I will look it later.

Also noticed that there are some lint errors according to the CI output, would you mind to fix them first?

Vigilans · 2019-01-23T05:31:08Z

The lint errors have been fixed.

Note: A default object is used to define IProblem type, so I suppressed the typedef requirement here:

// tslint:disable-next-line:typedef
export const IProblemDefault = {
    favorite: false,
    locked: false,
    state: ProblemState.Unknown,
    id: "",
    name: "",
    difficulty: "",
    passRate: "",
    companies: [] as string[],
    tags: [] as string[],
};

export type IProblem = typeof IProblemDefault;

The default object is then used to create a new IProblem with default properties:

new LeetCodeNode(Object.assign({}, list.IProblemDefault, {
    id: "Root",  // parent node name
    name: "Tag", // current node name
}), false),

jdneo · 2019-01-24T07:37:59Z

Hi @Vigilans,

Is there any reason to add accepted/not accepted categories into the explorer?

Vigilans · 2019-01-24T07:49:06Z

The Not-AC category shows problems which are submitted but not accepted.
For AC category, I wonder if there will be some mechanisms like retrieving all accepted codes of one problem locally or remotely. For now, this category seems redundant...

If you think them not necessary, I will try to revert this commit.

jdneo · 2019-01-24T08:02:06Z

Hmm, I think it's a little bit redundant somehow.

I better approach might be: having an extension setting that specifies if the explorer will only show unresolved problems or all the problems, then this setting will apply to all the categories.

To retrieving all accepted codes, this can be another action in the explorer context menu, which is another story.

This reverts commit 7e00062.

twchn · 2019-01-24T08:19:09Z

@jdneo Yeah, the feature I mentioned. 😄 #95

Vigilans · 2019-01-24T08:19:50Z

I see. Btw, there are some other changes I plan to contribute, maybe I should open another issue to query you for your opinions_(눈_ 눈」∠)_

Vigilans · 2019-01-24T08:21:30Z

@taoweicn I will work on this feature later.

twchn · 2019-01-24T08:24:23Z

@taoweicn I will work on this feature later.

👍 very thankful.

jdneo · 2019-01-24T08:30:27Z

@Vigilans Contributions are welcome! 👍

Yes Let's open issues and discuss the implementation there first before we create a PR. 😄

jdneo · 2019-01-24T09:07:23Z

@Vigilans Could you please send another PR about hiding solved problems?

A PR contains several issue fixes will make it too large to review

Vigilans · 2019-01-24T09:17:17Z

@jdneo The hiding solved feature is implemented with only 15 lines of code in 2 files, and is part of the tree view.
Personally I think there is not much burden in reviewing this feature, unless more requirements are introduced. Next time I will take care of it when committing new changes_(:зゝ∠)_

jdneo · 2019-01-24T12:34:44Z

src/commands/list.ts

+    tags: [] as string[],
+};
+
+export type IProblem = typeof IProblemDefault;


It would be better to put the IProblem definition and IProblemDefault to other places instead of in this file.
Let's put them into shared.ts, I'll refactor this part later.

jdneo · 2019-01-24T12:36:29Z

src/leetCodeExecutor.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT license.

 import * as cp from "child_process";
+import * as fs from "fs";


Use fs-extra

jdneo · 2019-01-24T12:38:05Z

src/leetCodeExecutor.ts

+    public async getCompaniesAndTags(): Promise<{ companies: { [key: string]: string[] }, tags: { [key: string]: string[] } }> {
+        // preprocess the plugin source
+        const componiesTagsPath: string = path.join(await leetCodeExecutor.getLeetCodeRootPath(), "lib", "plugins", "company.js");
+        const componiesTagsSrc: string = (await fs.readFileSync(componiesTagsPath, "utf8")).replace(


We can use readFile from fs-extra.

Something went wrong with my usage. fs-extra is correct.

jdneo · 2019-01-24T12:39:56Z

src/leetCodeExecutor.ts

+            "module.exports = { COMPONIES, TAGS }",
+        );
+        // require plugin from modified string
+        const requireFromString: (src: string, path: string) => any = require("require-from-string");


Can we import the module?

For example, npm i --save-dev @types/require-from-string
Then import it instead of using require

Yeah, the definitelyTyped indeed provides type for require-from-string.
import * as requireFromString from "require-from-string"; successfully worked.

jdneo · 2019-01-24T12:40:50Z

src/leetCodeExplorer.ts

@@ -7,6 +7,8 @@ import * as list from "./commands/list";
 import { leetCodeManager } from "./leetCodeManager";
 import { ProblemState } from "./shared";

+export type Category = "Difficulty" | "Tag" | "Company";


Can we use enum instead?

The LeetCodeTreeDataProvider.treeData needs the type semantic from Category to preserve the type hint, whereas enum just provides the value semantic.

Without the string union type, this line won't get proper type hint:

const problems: list.IProblem[] | undefined = this.treeData[parent].get(subCategory);

this.treeData[parent] will return any type.

If we change Category to enum type, then the above line should be rewritten to:

this.treeData[parent.toString() as "Difficulty" | "Tag" | "Company"]

which falls back to where it starts.

Have you tried this:

export enum Category { Difficulty = "Difficulty", Tag = "Tag", Company = "Company", }

After trying, Category.Tag.toString() only returns type string.

There is no one-line way to get the type hint here, reference:
https://stackoverflow.com/questions/52370544/parse-string-as-typescript-enum/
https://stackoverflow.com/questions/52393730/typescript-string-literal-union-type-from-enum

No need to call .toString() here.

Whenever you want to use the Category value as a string, just using Category.xxxx.

By doing this, you will get the value as a string meanwhile keep the typedef.

I tried using enum here and everything works fine.

Let's make the Category enum type:

export enum Category { Difficulty = "Difficulty", Tag = "Tag", Company = "Company", }

Then rewrite line 147 this way:

const categories: Map<Category, string[]> = new Map<Category, string[]>([ [Category.Difficulty, [problem.difficulty]], [Category.Tag, problem.tags], [Category.Company, problem.companies], ]);

Follows the iteration:

for (const [parent, children] of categories) { for (let subCategory of children) {

Here, typeof parent is Category:

Continues the line 156:

const problems: list.IProblem[] | undefined = this.treeData[parent].get(subCategory);

Since typeof parent is Category and this.treeData's key type is string, get's type could not be deduced from the usage, and there's no way to use Category.xxxx here:

Meanwhile, we couldn't change this.treeData type to Map<Category, Map<string, list.IProblem[]>> since there is a field Favorite which doesn't processed as Category and its value type is list.IProblem[].

If you insist to use enum here, a workaround could be:

private treeData: { [Category.Difficulty]: Map<string, list.IProblem[]>, [Category.Tag]: Map<string, list.IProblem[]>, [Category.Company]: Map<string, list.IProblem[]>, Favorite: list.IProblem[], };

Now get's type could be deduced properly:

Yet I think it complicates what could have been an easy and elegant solution in Typescript.

We need to write more hard-coded strings if using string union here while using enum can get rid of this. That's the reason I prefer using enum here: make the code easier to maintain in the future.

There are many ways to get rid of the special field Favorite. One simple way to do a simple check before getting the map. TS will help us to infer the type during compilation.

jdneo · 2019-01-24T12:48:05Z

src/leetCodeExplorer.ts

+            Company: new Map(),
+            Favorite: [],
+        };
+        for (const problem of await list.listProblems()) {


I'm thinking that we can filter the accepted problems in the method list.listProblems(). If hideSolved === true then only return unaccepted problems.

By doing this, we decouple the this.leetCodeConfig with the explorer

In my opinion, Favorite group should show all starred problems no matter whether it's accepted or not.

Maybe it could be checked this way in list.listProblems():
!problem.favorite && problem.state === ProblemState.AC && hideSolved -> continue
The flaw is that the favorite checking logic is duplicated.

Hmm, ok, just keep it there first.

jdneo · 2019-01-24T12:50:51Z

src/leetCodeExplorer.ts

+                continue;
+            }
+            // Add problems according to category
+            const categories: Array<[Category, string[]]> = [


Can we use Map here?

use index to represent the parent-children relationship is confusing

We could use Map here this way:

const categories: Map<Category, string[]> = new Map<Category, string[]>([ ["Difficulty", [problem.difficulty]], ["Tag", problem.tags], ["Company", problem.companies], ]);

Btw, a better way would be to use Object.entries here, but it is not supported by es6, which is specified by tsconfig.json.

Let's use Map here.

jdneo · 2019-01-24T12:53:03Z

src/leetCodeExplorer.ts

+            for (const [parent, children] of categories) {
+                for (let subCategory of children) {
+                    // map 'first-second' to 'First Second'
+                    subCategory = subCategory.split("-").map((c: string) => c[0].toUpperCase() + c.slice(1)).join(" ");


first-second is not that bad.

Cuz current logic could handle a-b but not a-b-c

Just leave first-second as it is for now.

What does a-b-c refer to? The code here could transform breadth-first-search to Breadth First Search.

My bad, it's fine.

jdneo · 2019-01-24T12:58:27Z

src/leetCodeExplorer.ts

@@ -64,57 +73,100 @@ export class LeetCodeTreeDataProvider implements vscode.TreeDataProvider<LeetCod
        }

        const idPrefix: number = Date.now();
-        return {
+        return { // element.id -> parent node name, element.name -> node name


Why we save parent node name into element.id?

There should be some field to store the parent node name when it is not problem node...since in previous commits you simply set node.id and node.name the same value, I decided to reuse the id field to represent some other value.

The reasons for the necessity of parent node name are:

We should be able to retrieve data from the LeetCodeTreeDataProvider.treeData.

One problem will now be added as LeetCodeNode at least 3 times(in Difficulty, Tag and Company), we should be able to distinguish them, hence the following naming rule:

id: `${idPrefix}.${element.id}.${element.name}`,

(idPrefix = Date.now() just returns same value throughout initialization process)

Maybe we could add a filed in the LeetCodeNode called 'parentName' to store the parent node info.

jdneo

Will review later

jdneo · 2019-02-02T05:36:57Z

Merged with some refactoring.

@Vigilans Thank you for your contribution. The idea is very inspiring!

… & Favorite (LeetCode-OpenSource#94)

Vigilans added 9 commits January 22, 2019 20:20

Add company plugin check

e90b44e

Store leetcode-cli root path in LeetCodeExecutor instead of binary path

c1a58b2

Add companies and tags information to IProblem

ce204b9

(from direct import of leetcode-cli company plugin)

Add category tree view to explorer

236a020

Add require-from-string dependency

64badc5

Move getCompaniesAndTags from list.ts to leetCodeExecutor.ts

d648f8a

Add unknown label to companies and tags

Minor fixes

21723fe

Capitalize label name and sort labels

39071bf

Add favorite tree view

3cb8743

Fix lint errors

86f17ac

Show AC/NotAC problems in treeview

7e00062

Revert "Show AC/NotAC problems in treeview"

0153d84

This reverts commit 7e00062.

Option to hide solved problems

d9abd5f

jdneo reviewed Jan 24, 2019

View reviewed changes

jdneo reviewed Feb 2, 2019

View reviewed changes

Refactor

7bffd54

jdneo merged commit 24a59a3 into LeetCode-OpenSource:master Feb 2, 2019

This was referenced Feb 2, 2019

Hide solved problems #95

Closed

Support label search, for example: array, map, greedy algorithms... #67

Closed

Vigilans deleted the tag-search branch March 14, 2019 06:15

ringcrl pushed a commit to ringcrl/vscode-leetcode that referenced this pull request Apr 21, 2019

Support categorize the problem according to: Company, Difficulty, Tag…

83ae7ec

… & Favorite (LeetCode-OpenSource#94)

Enhanced Tree view #94

Enhanced Tree view #94

Conversation

Vigilans commented Jan 23, 2019 • edited Loading

Introduction

Details

Demonstration

jdneo commented Jan 23, 2019

Vigilans commented Jan 23, 2019 • edited Loading

jdneo commented Jan 24, 2019

Vigilans commented Jan 24, 2019 • edited Loading

jdneo commented Jan 24, 2019

twchn commented Jan 24, 2019

Vigilans commented Jan 24, 2019 • edited Loading

Vigilans commented Jan 24, 2019

twchn commented Jan 24, 2019

jdneo commented Jan 24, 2019

jdneo commented Jan 24, 2019

Vigilans commented Jan 24, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vigilans Jan 31, 2019 • edited Loading

Choose a reason for hiding this comment

jdneo Feb 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vigilans Jan 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vigilans Jan 24, 2019 • edited Loading

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

jdneo left a comment • edited Loading

Choose a reason for hiding this comment

jdneo commented Feb 2, 2019 • edited Loading

Vigilans commented Jan 23, 2019 •

edited

Loading

Vigilans commented Jan 23, 2019 •

edited

Loading

Vigilans commented Jan 24, 2019 •

edited

Loading

Vigilans commented Jan 24, 2019 •

edited

Loading

Vigilans Jan 31, 2019 •

edited

Loading

jdneo Feb 2, 2019 •

edited

Loading

Vigilans Jan 24, 2019 •

edited

Loading

Vigilans Jan 24, 2019 •

edited

Loading

jdneo left a comment •

edited

Loading

jdneo commented Feb 2, 2019 •

edited

Loading