-
Notifications
You must be signed in to change notification settings - Fork 62
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
feat(participant): export to a playground VSCODE-574 #832
Conversation
…SCODE-574-export-to-playground
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, left a couple comments mostly on prompt suggestions. Stoked to give this a try.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, a couple small nit comments and one on our promise usage and error handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done - I mostly have minor comments/suggestions.
token, | ||
}), | ||
new Promise<undefined>((resolve) => | ||
token.onCancellationRequested(() => { | ||
resolve(undefined); | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're already passing the cancellation token to getChatResponseContent
- why do we need the extra promise here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem that the token passed to the await model.sendRequest(messages, {}, token)
terminates the request to the model and the model returns a result anyway. I want to investigate this more separately and maybe file the ticket to vscode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left two suggestions/questions, not blockers. lgtm! Haven't tried it out much yet
…SCODE-574-export-to-playground # Conflicts: # src/participant/participant.ts # src/test/suite/participant/participant.test.ts
…playground # Conflicts: # src/editors/playgroundController.ts # src/participant/participant.ts # src/participant/prompts/schema.ts # src/telemetry/telemetryService.ts # src/test/suite/editors/playgroundController.test.ts # src/test/suite/participant/participant.test.ts # src/test/suite/telemetry/telemetryService.test.ts
…playground # Conflicts: # src/participant/prompts/index.ts
Description
Convert the full text from the active editor or selected lines of code written in the input language to the Shell language and open a new MongoDB playground with the generated code.
Checklist
Motivation and Context
Types of changes