Skip to content

feat: support load code from local file for issue 59 #149

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

Merged
merged 2 commits into from
Feb 24, 2019

Conversation

poppinlp
Copy link
Contributor

This RP is for #59. The main logic change is that showProblem operation will try to load local code first. Here are the changes:

  • Add lodash.kebabcase in dependencies to generate a slug
  • Add langExt in shared.ts to do the extension mapping for language
  • Add problemUtils.ts which contains utils for problem
  • The showProblem in leetCodeExecutor.ts will return the code file path right now

export function genFileName(node: IProblem, language: string): string {
const name: string = kebabCase(node.name);
const ext: string | undefined = langExt.get(language);
return `${node.id}.${name}.${ext}`;
Copy link
Member

Choose a reason for hiding this comment

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

ext here might be undefined. Though according to current logic, it's impossible.

I think you can have another util method here to parse the ext from a language name. Meanwhile, throw an error if the language is not supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I'll fix it.

@@ -0,0 +1,8 @@
import kebabCase = require("lodash.kebabcase");
Copy link
Member

Choose a reason for hiding this comment

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

Use import instead of require.

Copy link
Contributor Author

@poppinlp poppinlp Feb 22, 2019

Choose a reason for hiding this comment

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

It's because of the issue of lodash. T_T
Another way is to change the tsconfig.json. But I think its impact may be even bigger, so I wrote it like this.

Copy link
Member

Choose a reason for hiding this comment

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

OK, it's fine.

public async showProblem(node: IProblem, language: string, outDir: string): Promise<string> {
const fileName: string = genFileName(node, language);
const filePath: string = path.join(outDir, fileName);
const hasLocalCode: boolean = await fse.pathExists(filePath);
Copy link
Member

Choose a reason for hiding this comment

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

hasLocalCode is redundant. We can move await fse.pathExists(filePath) into the if block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK.

Copy link
Member

@jdneo jdneo left a comment

Choose a reason for hiding this comment

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

Don't forget to fix the linting error.

Overall the idea is clear and promising!

@poppinlp
Copy link
Contributor Author

I'm sorry for the lint error. T_T

Copy link
Member

@jdneo jdneo left a comment

Choose a reason for hiding this comment

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

LGTM

@jdneo jdneo merged commit 7ca9b8a into LeetCode-OpenSource:master Feb 24, 2019
@jdneo
Copy link
Member

jdneo commented Feb 24, 2019

@poppinlp Thank you for the contribution. Nice job!

I'll send out another PR to add you the contributors..

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