feat: implement restore tab in chat client #933
Conversation
vidkhath
force-pushed
the
vidkhath/open-tab
branch
from
ec67849 to
6e206af
Compare
client/vscode/src/chatActivation.ts
Outdated
| }, 30000) | ||
|
|
||
| const messageHandler = (message: any) => { | ||
| if (typeof webviewMessageId !== 'string' && message.webviewMessageId === webviewMessageId) { |
There was a problem hiding this comment.
How is it working? Don't we need to pass webviewMessageId back from chat.ts?
There was a problem hiding this comment.
It wasnt :D Fixed it in a new revision
client/vscode/src/chatActivation.ts
Outdated
| reject(error) | ||
| } | ||
|
|
||
| webviewView.webview.onDidReceiveMessage(messageHandler, errorHandler) |
There was a problem hiding this comment.
This returns a disposable. Should we dispose it given there will be multiple registrations in this flow?
There was a problem hiding this comment.
Yes, that makes sense. Let me do that in a new revision
vidkhath
force-pushed
the
vidkhath/open-tab
branch
from
6e206af to
9549cde
Compare
| @@ -179,16 +178,15 @@ export class Messager { | |||
| this.chatApi.telemetry({ ...params, name: ERROR_MESSAGE_TELEMETRY_EVENT }) | |||
| } | |||
|
|
|||
| onOpenTab = (result: OpenTabResult | ErrorResult): void => { | |||
| this.chatApi.onOpenTab(result) | |||
| onOpenTab = (requestId: string, result: OpenTabResult | ErrorResult): void => { | |||
There was a problem hiding this comment.
Can we also add a test that requestId was propagated from inbound to outbound message? Can be separate PR.
| const disposable = webviewView.webview.onDidReceiveMessage((message: any) => { | ||
| if (message.requestId === requestId) { | ||
| clearTimeout(timeout) | ||
| disposable.dispose() |
There was a problem hiding this comment.
Do we also need to dispose in case of reject?
| @@ -54,6 +54,7 @@ export type ServerMessageCommand = | |||
|
|
|||
| export interface Message { | |||
There was a problem hiding this comment.
nit: it's not clear why we have Message as a separate interface - you could merge it with ServerMessage.
| @@ -224,9 +224,10 @@ export const createChat = ( | |||
| disclaimerAcknowledged: () => { | |||
| sendMessageToClient({ command: DISCLAIMER_ACKNOWLEDGED }) | |||
| }, | |||
| onOpenTab: (params: OpenTabResult | ErrorResult) => { | |||
| onOpenTab: (requestId: string, params: OpenTabResult | ErrorResult) => { | |||
There was a problem hiding this comment.
We need to explain what requestId is somewhere. e.g. README
There was a problem hiding this comment.
Agreed, I will add it in the README
vidkhath
force-pushed
the
vidkhath/open-tab
branch
from
9549cde to
c2829ba
Compare
Problem
Extend open tab chat client implementation to display historic chat to restore a tab
Solution
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.