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

Allow to specify title and placeholder in API for interpreter quickpick #19896

Merged
merged 2 commits into from
Sep 27, 2022

Conversation

karrtikr
Copy link

For #19891

@karrtikr karrtikr added no-changelog No news entry required skip tests Updates to tests unnecessary labels Sep 27, 2022
@@ -47,7 +47,7 @@ export interface IQuickPickParameters<T extends QuickPickItem, E = any> {
canGoBack?: boolean;
items: T[];
activeItem?: T | Promise<T>;
placeholder: string;
placeholder: string | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

For Tech debt, we can probably get rid of multistepinput.ts

Copy link
Author

Choose a reason for hiding this comment

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

Rid of in favor of what 🤔 we're still using it in many places. But yeah it could use some cleanup.

Copy link
Member

Choose a reason for hiding this comment

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

Replacing the DI with calling raw vscode APIs. I can see the use for a wrapper function, but it is a state less class.

Copy link
Author

Choose a reason for hiding this comment

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

This has internal states though, maybe they'll need to be passed in.

@karrtikr karrtikr merged commit 870ae8f into microsoft:main Sep 27, 2022
@karrtikr karrtikr deleted the params branch September 27, 2022 18:32
eleanorjboyd pushed a commit to eleanorjboyd/vscode-python that referenced this pull request Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog No news entry required skip tests Updates to tests unnecessary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants