Skip to content

feature: Added an option to disable endpoint translation #690

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 8 commits into from
May 31, 2021

Conversation

dzz007
Copy link
Contributor

@dzz007 dzz007 commented Mar 22, 2021

After my pull request gets merged on leetcode-cli (which provides the -T option to disable endpoint translation), this pull requests will give user an option to toggle in settings whether or not they want to use endpoint's translation for the problems.

dzz007 added 3 commits March 21, 2021 22:10
Because leetcode-cli now will automatically clear cache
 if it detects a change in the -T flag.
@dzz007
Copy link
Contributor Author

dzz007 commented Apr 10, 2021

@jdneo Thank you for bumping leetcode-cli to 2.8.0! Do you mind publish that change to npmjs as well? So I can specify the new version in package.json. Or do you prefer to specify the commit hash in package.json?

@jdneo
Copy link
Member

jdneo commented Apr 11, 2021

@dzz007 you can first use the latest bit of the cli and link it to the vs code extension locally. Let me know if everything works fine. After that I'll release it to the registry.

@dzz007
Copy link
Contributor Author

dzz007 commented Apr 11, 2021

@jdneo thank you for responding! I just tested with locally installing with master branch of leetcode-cli in vscode-leetcode. Everything works fine, I checked both the leetcode.com and leetcode-cn.com endpoints.
The only problem I found is that changing Endpoint in vscode-leetcode's setting page actually doesn't work. But that is a separate bug which is not related to this leetcode-cli upgrade at all, and I verified that because it exists in the current version as well.

@jdneo
Copy link
Member

jdneo commented Apr 11, 2021

Just published 2.8.0.

@dzz007
Copy link
Contributor Author

dzz007 commented Apr 11, 2021

@jdneo Thanks, just finished changing package.json & package-lock.json. I tested again and it works well.

@jdneo
Copy link
Member

jdneo commented Apr 11, 2021

The content of package-lock.json looks strange, why the entries have node_modules prefix?

@dzz007
Copy link
Contributor Author

dzz007 commented Apr 11, 2021

I believe it's because I am currently using NPM v7 which introduced some changes to the lock file as specified in "lockfileVersion": 2. I can downgrade to NPM v6 and regenerate the lock file if you want.

@dzz007
Copy link
Contributor Author

dzz007 commented Apr 11, 2021

Just a sidenote, IIRC v2 version of the lock-file is backward compatible with older version of NPM. So it should be able to work fine with NPMv6 for example.

@jdneo
Copy link
Member

jdneo commented Apr 11, 2021

ok, thanks for the information

@dzz007
Copy link
Contributor Author

dzz007 commented Apr 11, 2021

@jdneo no problem! Do you want me to keep the lock file as-is or do you want or to use the old version?

@jdneo
Copy link
Member

jdneo commented Apr 12, 2021

If it's backward compatible, then it's fine I think.

@dzz007
Copy link
Contributor Author

dzz007 commented Apr 13, 2021

Thank you for your review! I just added the application scope.

yihong0618
yihong0618 previously approved these changes Apr 14, 2021
@dzz007 dzz007 requested a review from jdneo April 14, 2021 17:56
@jdneo
Copy link
Member

jdneo commented Apr 15, 2021

Coule you add a tsdoc for the showSolution function notifying the param needTranslation will not take effect?

@dzz007
Copy link
Contributor Author

dzz007 commented Apr 15, 2021

@jdneo That's a good point! I will add it in couple hours. Do you want me to put it in leetCodeExecutor?

@dzz007
Copy link
Contributor Author

dzz007 commented Apr 20, 2021

Thanks!

@jdneo jdneo merged commit 0fcf9f1 into LeetCode-OpenSource:master May 31, 2021
@jdneo jdneo added this to the 0.17.1 milestone May 31, 2021
@jdneo
Copy link
Member

jdneo commented May 31, 2021

Apologize for lazy merge. I just get the permission to merge the PRs. 😃

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.

3 participants