Skip to content

add switchDefaultLanguage #114

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

Closed
wants to merge 3 commits into from
Closed

add switchDefaultLanguage #114

wants to merge 3 commits into from

Conversation

ZhangYet
Copy link

Once you choose the default language, every time you show the new problem with the default language. If you want to change another language, you need to change it in Preference. I think it's convenient for a command to switch default language.

@jdneo
Copy link
Member

jdneo commented Feb 11, 2019

Hi @ZhangYet

Thank you for your contribution.

May I ask the background about this PR first? Because I want to better understand how users are using this extension.

Does that mean each time you want to practice a problem, you would like to practice it with multiple languages? How frequently will the 'switching default language' happen?

@jdneo jdneo self-requested a review February 11, 2019 02:28
@ZhangYet
Copy link
Author

I use Go and Python in my job, so I practice leetcode with go and python. And now I'm going to learn rust so I would like to answer problems with rust.

However, I'm not sure if anyone else wants to switch their default language frequently.

@jdneo
Copy link
Member

jdneo commented Feb 11, 2019

@ZhangYet Got it.

So from my understanding, you are used to practicing one language for a period, and maybe another language later. During a period of time, you still prefer using one default language rather than selecting the language explicitly each time when picking a problem. Am I right?

@ZhangYet
Copy link
Author

@jdneo In fact, I will switch to python if I have trouble when using other languages.
Anyway if you think it's not a practical feature, you can close this PR. I agree that maybe few people would use this feature

@jdneo
Copy link
Member

jdneo commented Feb 11, 2019

@ZhangYet Hmm, Actually I do not want to close this PR for now, cuz we do not know if other users need this feature as well or not. Instead, I create a tracking issue #115. If any user wants this, they can click the 👍 to let us know.

Back to the problem you meet now, the workaround is to remove the default language setting, then the extension will ask you to select the language each time you pick a problem and click something like Never show again when the extension asks you to set the default language.

At last, I still want to thank you for sending out this PR. 😄 Let's wait for a while to see the users' feedback.

@ZhangYet
Copy link
Author

@jdneo LGTM tks

@jdneo
Copy link
Member

jdneo commented Feb 19, 2019

@ZhangYet There are several users click 👍 in the related issue. I think we can start working on this feature now.

import { DialogType, promptForOpenOutputChannel, promptForSignIn } from "../utils/uiUtils";
import { deleteCache } from "./cache";

export async function switchDefaultLanguage(): Promise<void> {
const leetCodeConfig: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration("leetcode");
Copy link
Member

Choose a reason for hiding this comment

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

The code here is somehow duplicated with https://github.com/jdneo/vscode-leetcode/blob/master/src/commands/show.ts#L44-L51

Can we extract them out to share only one piece of code?

import { DialogType, promptForOpenOutputChannel, promptForSignIn } from "../utils/uiUtils";
import { deleteCache } from "./cache";

export async function switchDefaultLanguage(): Promise<void> {
const leetCodeConfig: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration("leetcode");
const language: string | undefined = await vscode.window.showQuickPick(languages, { placeHolder: "Select the language you want to use" });
Copy link
Member

Choose a reason for hiding this comment

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

If the user has already set a default language, we should at least add a ✅mark before the language.

Copy link
Author

Choose a reason for hiding this comment

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

I will do it later for I am going to change my job these days.

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.

@ZhangYet Some comments

@jdneo
Copy link
Member

jdneo commented Feb 22, 2019

@ZhangYet Would you like to keep working on this PR?

@ZhangYet
Copy link
Author

@jdneo Fine. I will fix the conflict.

@jdneo
Copy link
Member

jdneo commented Mar 2, 2019

Fixed in #178

@ZhangYet Still want to thank you for the engagement and the inspiring feature request 👍

@jdneo jdneo closed this Mar 2, 2019
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