-
Notifications
You must be signed in to change notification settings - Fork 748
feat(chat): Add header for shell command #6939
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(chat): Add header for shell command #6939
Conversation
|
| await this.handleCreatePrompt(message) | ||
| break | ||
| case 'accept-code-diff': | ||
| case 'confirm-tool-use': |
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.
Do we need confirm-tool-use ?
Can this be replaced with run-shell-command?
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.
can remove it in next pr.
| break | ||
| case 'run-shell-command': | ||
| answer.header = { | ||
| body: 'shell', |
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 think we can follow similar approach as this for storing body and icon values from messenger.ts.
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.
not sure what you want me to follow?
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.
If possible, Can we pass values of body and icon etc in this approach?
| status: 'info', | ||
| }) | ||
| let shellCommandHeader = undefined | ||
| if (toolUse?.name === ToolType.ExecuteBash && message.startsWith('```shell')) { |
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.
Do we need this check ?
message.startsWith('```shell')
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.
yes, this is needed, as the execution log also has the ToolType.ExecuteBash.
|
from failing CI: |
|
65791e5 to
69de65c
Compare
Problem
Solution
feature/xbranches will not be squash-merged at release time.