Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feature: Added support for opting out endpoint translation #56

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
jdneo merged 3 commits into leetcode-tools:master from dzz007:develop
Apr 8, 2021

Conversation

@dzz007
Copy link

@dzz007 dzz007 commented Mar 22, 2021

Added a new parameter '-T' which when specified in 'show', 'list', or 'submission', will disable the end point's translation to problem materials. This can be used by vscode-leetcode to allow user to opt out from translations. I will push a separate pull request to that repo later when the version on this repo is bumped.

Copy link

I changed to branch develop.
But I don't know how to use -T in list or show
Can you give me some examples ?

Copy link
Author

dzz007 commented Mar 29, 2021
edited
Loading

I changed to branch develop.
But I don't know how to use -T in list or show
Can you give me some examples ?

Thank you for your comment! The idea of -T is to provide vscode-leetcode the ability to let user choose whether or not they would like to see untranslated questions. This will allow users to see english version of the questions even though they are on the leetcode-cn endpoint.

Because of the existence of caching mechanism, switching between -T and no -T require doing a ./leetcode cache -d.

An example would be:
<---->
./leetcode cache -d
./leetcode list -T <= this gives english version, because -T means don't translate
<---->
./leetcode cache -d <= delete cache first, this will only be required if you are switching between -T and non -T
./leetcode list <= this gives translated (chinese) questions. By default -T is not set so it will use end point's question translation.

Copy link

yihong0618 commented Mar 29, 2021
edited
Loading

I changed to branch develop.
But I don't know how to use -T in list or show
Can you give me some examples ?

Thank you for your comment! The idea of -T is to provide vscode-leetcode the ability to let user choose whether or not they would like to see untranslated questions. This will allow users to see english version of the questions even though they are on the leetcode-cn endpoint.

Because of the existence of caching mechanism, switching between -T and no -T require doing a ./leetcode cache -d.

An example would be:
<---->
./leetcode cache -d
./leetcode list -T <= this gives english version, because -T means don't translate
<---->
./leetcode cache -d <= delete cache first, this will only be required if you are switching between -T and non -T
./leetcode list <= this gives translated (chinese) questions. By default -T is not set so it will use end point's question translation.

Can we clean the cache or only the problems cache in the code that user need not to clear cache first?

maybe add a flag isT or something in the cache

if(-T) {
 clearCache()
 doSomething()
}

Copy link
Author

dzz007 commented Mar 29, 2021

That may not be a good idea, usually a user might want to stick to a setting for a fairly long period of time. That means if one wants to use -T, he/she may stick with -T for quite a while, so clearing the cache internally would render the cache obsolete.
I have a commit ready that adds an option checkbox on the vscode-leetcode plugin which basically allow the user to add -T every time so they can stick with a specific language.

Copy link

yihong0618 commented Mar 29, 2021
edited
Loading

That may not be a good idea, usually a user might want to stick to a setting for a fairly long period of time. That means if one wants to use -T, he/she may stick with -T for quite a while, so clearing the cache internally would render the cache obsolete.
I have a commit ready that adds an option checkbox on the vscode-leetcode plugin which basically allow the user to add -T every time so they can stick with a specific language.

That's why I think we can add one flag or something else to solve it.
Because people may already have cache already.

Copy link
Author

dzz007 commented Mar 29, 2021

Ohh sorry I missed that line. It's a good idea, we can probably make it so that if the cache sees the translation mode has changed it will automatically invalidate it. But adding this flag might cause some compatibility issues, I will take a closer look tomorrow.

yihong0618 reacted with thumbs up emoji

Copy link
Author

dzz007 commented Mar 30, 2021

@yihong0618 I just made a new commit which made some modification to the cache module. Now it will invalidate all caches automatically if the the old cache's -T config is different than requested. Also can you please take a look again at #55 , I fixed the const problem as well.
Thanks

Copy link

Will take a look and test later today.

dzz007 added 3 commits March 30, 2021 22:09
Fixed some translation issue
Continue to fix some translation problem
fix: fixed test cases
Fixed all test cases relating to getProblem(s) to adapt new parameter
fix: fixed test_leetcode testcase
Copy link
Author

dzz007 commented Mar 31, 2021

Thanks, I just rebased this branch onto the latest master, to make merge easier.

Copy link

yihong0618 commented Mar 31, 2021
edited
Loading

LGTM for now.

But one question:
How do we handle it in vscode-leetcode side?
If use -T, user should -T every time? Or do we keep it as default?

Copy link
Author

dzz007 commented Mar 31, 2021

Thank you for the review!
I have another PR(currently draft mode) on vscode-leetcode right here: LeetCode-OpenSource/vscode-leetcode#690

It's implemented already, I added a new checkable option in the setting page. Once this repo is upgraded in npm, I can open that PR.

Copy link

Got it. Thanks.

Copy link
Author

dzz007 commented Apr 6, 2021

@jdneo Hi, can you help me and take a look at this PR please? It provides a new option in the cli tool which later frontend can rely on to give users the ability to skip end point translations.

Copy link

jdneo commented Apr 7, 2021

@dzz007 Is this used to specify the language of leetcode-cn (Chinese -> English), how about leetcode US?

Copy link
Author

dzz007 commented Apr 7, 2021

@jdneo Thanks for replying, yes, for the leetcode US it won't have any impact at all. I tested it and it works fine for US endpoint.

Copy link

jdneo commented Apr 8, 2021

Ok, we should always be careful when adding new argument (for future compatibility and extensibility)

Currently, -T will translate Chinese to English for LeetCode CN, what if in the future a new language appear? (though I'm not sure whether LeetCode will plan to do it or not). Do we need to follow with the target language as a argument?

Copy link
Author

dzz007 commented Apr 8, 2021

Umm, I don't think it need an argument, currently the way this -T works is that:

  1. without -T [this is the default mode]
    it will automatically uses endpoint's translation, [so in future if leetcode has French endpoint, by default it would display French].
  2. with -T
    it will disable endpoint's translation, which will return English version regardless which endpoint you are on

Copy link

jdneo commented Apr 8, 2021

Ok makes sense. I'll take a look later today.

@jdneo jdneo merged commit dfbb5b1 into leetcode-tools:master Apr 8, 2021
Copy link

jdneo commented Apr 8, 2021

Thanks @dzz007!

Copy link
Author

dzz007 commented Apr 8, 2021

Thanks! @jdneo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdneo jdneo jdneo approved these changes

+1 more reviewer

@yihong0618 yihong0618 yihong0618 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /