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

Polish query accepting #183995

Merged
merged 1 commit into from May 31, 2023
Merged

Polish query accepting #183995

merged 1 commit into from May 31, 2023

Conversation

TylerLeonhardt
Copy link
Member

We shouldn't send the same request to Copilot if the query hasn't changed. So if the query is the same, we short circut.

Fixes https://github.com/microsoft/vscode-internalbacklog/issues/4286

Also, when we open in chat, we should use the last accepted query, not what's in the input box.

Fixes https://github.com/microsoft/vscode-internalbacklog/issues/4280

We shouldn't send the same request to Copilot if the query hasn't changed. So if the query is the same, we short circut.

Fixes microsoft/vscode-internalbacklog#4286

Also, when we open in chat, we should use the last accepted query, not what's in the input box.

Fixes microsoft/vscode-internalbacklog#4280
@TylerLeonhardt TylerLeonhardt self-assigned this May 31, 2023
@TylerLeonhardt TylerLeonhardt enabled auto-merge (squash) May 31, 2023 21:44
@Tyriar Tyriar disabled auto-merge May 31, 2023 21:51
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Disabled auto merge in case you want the optional suggestions here

@@ -93,8 +94,9 @@ class AskQuickQuestionAction extends Action2 {
});
disposableStore.add(openInChat);
disposableStore.add(openInChat.onChange(async () => {
await this._currentSession?.openInChat(this._input!.value);
await this._currentSession?.openInChat(this._lastAcceptedQuery ?? this._input!.value);
Copy link
Member

Choose a reason for hiding this comment

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

You could avoid the ! if you set a input variable to the outer function scope

Copy link
Member Author

Choose a reason for hiding this comment

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

will add in debt week

@@ -249,10 +253,13 @@ class InteractiveQuickPickSession extends Disposable {

async accept(query: string) {
await this._model.waitForInitialization();
const requests = this._model.getRequests();
const lastRequest = requests[requests.length - 1];
if (lastRequest?.message && lastRequest?.message === query) {
Copy link
Member

Choose a reason for hiding this comment

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

The lastRequest?.message condition is redundant

Copy link
Member Author

Choose a reason for hiding this comment

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

will add in debt week

@TylerLeonhardt TylerLeonhardt merged commit c4cfd83 into main May 31, 2023
7 checks passed
@TylerLeonhardt TylerLeonhardt deleted the tyler/severe-alligator branch May 31, 2023 22:12
@github-actions github-actions bot locked and limited conversation to collaborators Jul 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants