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

feature: Added support for opting out endpoint translation #56

Merged
merged 3 commits into from
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.

@yihong0618
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 ?

@dzz007
Copy link
Author

dzz007 commented Mar 29, 2021

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.

@yihong0618
Copy link

yihong0618 commented Mar 29, 2021

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()
}

@dzz007
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.

@yihong0618
Copy link

yihong0618 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.

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

@dzz007
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.

@dzz007
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

@yihong0618
Copy link

Will take a look and test later today.

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
@dzz007
Copy link
Author

dzz007 commented Mar 31, 2021

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

@yihong0618
Copy link

yihong0618 commented Mar 31, 2021

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?

@dzz007
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.

@yihong0618
Copy link

Got it. Thanks.

@dzz007
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.

@jdneo
Copy link

jdneo commented Apr 7, 2021

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

@dzz007
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.

@jdneo
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?

@dzz007
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

@jdneo
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
@jdneo
Copy link

jdneo commented Apr 8, 2021

Thanks @dzz007!

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants