-
Notifications
You must be signed in to change notification settings - Fork 33
feat: extended protocol for conversation-based agents experience support #368
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
Changes from all commits
16b22b2
ca4c656
88356ba
4499daa
1edb709
08317ab
a612f0d
f52d358
e7b6517
f816e4f
b662593
00de1a1
72acdc2
717b786
f003317
fb74893
32f7b7e
1a6e58e
e7e4a9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,10 @@ import { | |
OPEN_TAB_REQUEST_METHOD, | ||
OpenTabParams, | ||
OpenTabResult, | ||
CHAT_UPDATE_NOTIFICATION_METHOD, | ||
FILE_CLICK_NOTIFICATION_METHOD, | ||
ChatUpdateParams, | ||
FileClickParams, | ||
} from './lsp' | ||
|
||
export const chatRequestType = new AutoParameterStructuresProtocolRequestType< | ||
|
@@ -81,3 +85,9 @@ export const followUpClickNotificationType = new ProtocolNotificationType<Follow | |
export const openTabRequestType = new ProtocolRequestType<OpenTabParams, OpenTabResult, never, void, void>( | ||
OPEN_TAB_REQUEST_METHOD | ||
) | ||
export const chatUpdateNotificationType = new ProtocolNotificationType<ChatUpdateParams, void>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this update existing chat messages in the current tab or add new ones? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add and update - depends on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok! No deletion. |
||
CHAT_UPDATE_NOTIFICATION_METHOD | ||
) | ||
export const fileClickNotificationType = new ProtocolNotificationType<FileClickParams, void>( | ||
FILE_CLICK_NOTIFICATION_METHOD | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import { | ||
OPEN_FILE_DIFF_NOTIFICATION_METHOD, | ||
OpenFileDiffParams, | ||
ProtocolNotificationType, | ||
ProtocolRequestType, | ||
SELECT_WORKSPACE_ITEM_REQUEST_METHOD, | ||
SelectWorkspaceItemParams, | ||
SelectWorkspaceItemResult, | ||
} from './lsp' | ||
|
||
export const selectWorkspaceItemRequestType = new ProtocolRequestType< | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What;s the difference between this and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The intent and protocol is different. We need to get selected items back in server. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: I think we usually run into issues when we use ProtocolRequestType and therefore usually use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What kind of issues? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When would the client send this? What is the use case? |
||
SelectWorkspaceItemParams, | ||
SelectWorkspaceItemResult, | ||
never, | ||
void, | ||
void | ||
>(SELECT_WORKSPACE_ITEM_REQUEST_METHOD) | ||
|
||
export const openFileDiffNotificationType = new ProtocolNotificationType<OpenFileDiffParams, void>( | ||
OPEN_FILE_DIFF_NOTIFICATION_METHOD | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,8 @@ import { | |
CancellationToken, | ||
GetSsoTokenParams, | ||
didChangeDependencyPathsNotificationType, | ||
openFileDiffNotificationType, | ||
selectWorkspaceItemRequestType, | ||
} from '../protocol' | ||
import { ProposedFeatures, createConnection } from 'vscode-languageserver/node' | ||
import { | ||
|
@@ -321,6 +323,9 @@ export const standalone = (props: RuntimeProps) => { | |
onDidDeleteFiles: params => lspConnection.workspace.onDidDeleteFiles(params), | ||
onDidRenameFiles: params => lspConnection.workspace.onDidRenameFiles(params), | ||
onUpdateConfiguration: lspServer.setUpdateConfigurationHandler, | ||
selectWorkspaceItem: params => | ||
lspConnection.sendRequest(selectWorkspaceItemRequestType.method, params), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Do we have to specify Same applies to openFileDiff There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen different versions used throughout the codebase. If you know what it impacts exactly, I can change. |
||
openFileDiff: params => lspConnection.sendNotification(openFileDiffNotificationType.method, params), | ||
}, | ||
window: { | ||
showMessage: params => lspConnection.sendNotification(ShowMessageNotification.method, params), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,9 @@ import { | |
DidChangeDependencyPathsParams, | ||
UpdateConfigurationParams, | ||
InlineCompletionWithReferencesParams, | ||
OpenFileDiffParams, | ||
SelectWorkspaceItemParams, | ||
SelectWorkspaceItemResult, | ||
} from '../protocol' | ||
|
||
// Re-export whole surface of LSP protocol used in Runtimes. | ||
|
@@ -136,6 +139,10 @@ export type Lsp = { | |
onDidDeleteFiles: (handler: NotificationHandler<DeleteFilesParams>) => void | ||
onDidRenameFiles: (handler: NotificationHandler<RenameFilesParams>) => void | ||
onUpdateConfiguration: (handler: RequestHandler<UpdateConfigurationParams, void, void>) => void | ||
selectWorkspaceItem: ( | ||
handler: RequestHandler<SelectWorkspaceItemParams, SelectWorkspaceItemResult | undefined | null, void> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we allow for undefined or null as a result? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Customer could click cancel in the dialog popup, something needs to be returned in this case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What operations trigger this dialog popup? Is there another (existing or new) request that this relates to? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Followup action called "Choose another folder in your workspace". This is needed to request customer select a folder for context. |
||
) => void | ||
openFileDiff: (params: OpenFileDiffParams) => void | ||
} | ||
window: { | ||
showMessage: (params: ShowMessageParams) => Promise<void> | ||
|
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.
I don't understand what this means. Does this return the file contents? Show it on screen?
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.
This returns URI to selected folder/file back to LSP server. I do not think we need to return any contents as LSP server has access to contents if needed. The purpose of this method is to give customer possibility to select.