Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add outputPath Config #123

Merged
merged 15 commits into from
Feb 16, 2019
Merged

add outputPath Config #123

merged 15 commits into from
Feb 16, 2019

Conversation

ringcrl
Copy link
Contributor

@ringcrl ringcrl commented Feb 13, 2019

add outputPath config for leetcode #119

Change Request,Need Help...Find a way to get a Problem difficulty...

README.md Outdated Show resolved Hide resolved
@jdneo
Copy link
Member

jdneo commented Feb 14, 2019

@ringcrl

I think there is still some misunderstanding here. The aim of add the setting leetcode.outputPath is to make it possible for the users to categorize the files as their wish. So instead of make it as a enum setting, we should make it as a plain string.

Let me explain more about it:

The setting of leetcode.outputPath has no default value, which means it is empty by default. And, by default, when the user click show problem, the file goes to the base path of the workspace folder.

If the user set some values to it, for example: 'happy' then the file goes to the folder called happy

Meanwhile, besides let the user customize the output folder, we will also provide some special values, which are, as you know, ${tag}, ${language}, ${difficulty}. This means the user needs to explicitly brace the value with ${}, then the extension will get to know that: Oh, this is not a general path. I need to traslate it to another value...

Do you get my point?

src/commands/show.ts Outdated Show resolved Hide resolved
@ringcrl
Copy link
Contributor Author

ringcrl commented Feb 14, 2019

Thank you very much, I have made changes.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/commands/show.ts Outdated Show resolved Hide resolved
src/commands/show.ts Outdated Show resolved Hide resolved
@ringcrl
Copy link
Contributor Author

ringcrl commented Feb 15, 2019

Thank you very much for these helpful suggestions.

src/commands/show.ts Outdated Show resolved Hide resolved
@@ -16,16 +17,17 @@ export async function showProblem(node?: LeetCodeNode): Promise<void> {
if (!node) {
return;
}
await showProblemInternal(node.id);
await showProblemInternal(node);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though type casting from LeetCodeNode to IProblem is fine. Let's add getter in LeetCodeNode to get the data, which is typed as IProblem.

It looks like I need to do some refactors here later after this PR, The definition of LeetCodeNode and IProblem is overlapped.

src/commands/show.ts Outdated Show resolved Hide resolved
src/commands/show.ts Outdated Show resolved Hide resolved
@ringcrl
Copy link
Contributor Author

ringcrl commented Feb 16, 2019

Add tag selector

@jdneo jdneo merged commit 1cdca10 into LeetCode-OpenSource:master Feb 16, 2019
@jdneo
Copy link
Member

jdneo commented Feb 16, 2019

@ringcrl Looks much better now.

I just did some changes and merged the PR.

Thank you for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants