utils: Add support for copilot user input #2072
Conversation
| return lmsInPreferredOrder?.at(0); | ||
| } | ||
|
|
||
| export function createPrimaryPromptToGetSingleQuickPickInput(picks: string[], relevantContext?: string): string { |
There was a problem hiding this comment.
I'm not too concerned about how optimal this is yet. I think when we do this for other commands, we should probably have a way to pass custom prompts since it may not be a generic one shoe fits all scenario.
There was a problem hiding this comment.
Hmm yeah idk how far I would want to take the custom prompts. I could see adding some custom info you can pass in like description or what question you are being asked but I think the general shape should remain the same.
| /** | ||
| * Wrapper class of several `vscode.window` methods that handle user input. | ||
| */ | ||
| export declare class AzExtUserInput implements IAzureUserInput { |
There was a problem hiding this comment.
Why do we need to export this now?
There was a problem hiding this comment.
In this PR I use AzExtUserInput to create a new instance for when the user goes back to edit. I could probably move it to have the logic and check everytime a go back error is called but it seemed a little overkill to do that instead of the just changing it when the edit button is clicked. But lmk if you would prefer that and I can change it
|
|
||
| public showOpenDialog(): Promise<vscodeTypes.Uri[]> { | ||
| // Throw this back to the user | ||
| throw new InvalidInputError(); |
There was a problem hiding this comment.
How jarring is it when the error gets thrown back and the user has to take control? I'm not sure if there's a great way to do it, but just curious what it currently looks like.
There was a problem hiding this comment.
Basically the InvalidInputError() doesn't get thrown to the user (or at least it shouldn't). It behaves similar to how GoBackError() does. The error is thrown in the background and then the user just see the pick that copilot couldn't answer pop up (and any subsequent picks).
There was a problem hiding this comment.
Does the webview that says Copiloting is generating the picks close?
There was a problem hiding this comment.
I don't think so at the moment. Since the webview lives only in the aca extension there isn't that good of a way to dispose of it here. I may can add a listener on the ACA side to see if I can dispose of it anytime that error is thrown. The view does get disgarded once the picks are all finished but yeah I totally forgot about that.
There was a problem hiding this comment.
Can we pass a reference to the webview whenever we make a copilot UI call?
| const jsonResponse: string = JSON.parse(response) as string; | ||
| this._onDidFinishPromptEmitter.fire({ value: jsonResponse }); | ||
| return jsonResponse; | ||
| } else if (options.value) { |
There was a problem hiding this comment.
Should this be an else if?
I thought it'd be more like:
- Prompt Copilot
- If response is invalid, then check if (options.value)
- If it exists, return that
- If response is valid, just return that
| return jsonResponse; | ||
| } catch { | ||
| // if copilot is unable to provide a response, fall back to the default value if it exists | ||
| if (options.value) { |
There was a problem hiding this comment.
Shouldn't this be outside of if(options.prompt)? If there's no prompt, shouldn't we default to options.value?
There was a problem hiding this comment.
I think we would want it in both places actually. Like if copilot can't give a response then default to the options.value and if there is not prompt then also default to options.value.
utils/src/wizard/AzureWizard.ts
Outdated
| @@ -156,6 +158,16 @@ export class AzureWizard<T extends (IInternalActionContext & Partial<types.Execu | |||
| step = this.goBack(step); | |||
| } | |||
| continue; | |||
| } else if (pe.errorType === 'InvalidInputError') { | |||
| // If this error is thrown it means copilot was unable to provide a valid response | |||
| // In order to re prompt the user we need to set the ui back to AzExtUserInput | |||
There was a problem hiding this comment.
| // In order to re prompt the user we need to set the ui back to AzExtUserInput | |
| // To let the user take over completing the wizard, we should initialize an AzExtUserInput with the existing context |
This adds a new
CopilotUserInputwhich is essentially just a Wrapper class of severalvscode.windowmethods that handle user input.I will add a PR soon in ACA that shows how this can be used but in essence if context.ui is set to a CopilotUserInput then anytime showQuickPick, showInputBox, etc. is called it will prompt copilot for an answer instead of showing the user.
If copilot is unable to generate a response or the response it gives does not match one of the quick picks as
InvalidInputErroris thrown and this will switch the context.ui back to an AzExtUserInput and the quick picks will work as normal instead of polling Copilot.canPickMany case has not been fully implemented yet since ACA doesn't have any canPickMany quick picks so I would need to test this using rg. For now I am just throwing it back to the user to make the choice.