continue icon indicating copy to clipboard operation
continue copied to clipboard

feat: implement the abort signal in `/edit` diff gen

Open Iamshankhadeep opened this issue 1 year ago • 8 comments

fixes #1012

Iamshankhadeep avatar Mar 26 '24 14:03 Iamshankhadeep

Deploy Preview for continuedev canceled.

Name Link
Latest commit d9a669127b910cb65813b7c005f83fdc1077f167
Latest deploy log https://app.netlify.com/sites/continuedev/deploys/66034ffad42e7f00089e28f3

netlify[bot] avatar Mar 26 '24 14:03 netlify[bot]

@Iamshankhadeep I think there might be a simpler solution as I'm looking at this.

There's already a built-in cancellation method for the webview protocol that is used here (https://github.com/continuedev/continue/blob/preview/extensions/vscode/src/webviewProtocol.ts#L347) which we could apply here (https://github.com/continuedev/continue/blob/preview/extensions/vscode/src/webviewProtocol.ts#L411) in order to allow cancellation from the webview.

And then to cancel with a keyboard shortcut, rather than adding a new one, I want to use the shortcut we already have (cmd+shift+backspace) which is implemented here. If you just make the above change first, I actually think that this line of code might already solve the problem with the command (I know we had this working before, and I think that is how)

So in summary, maybe we should just add this to here

sestinj avatar Apr 01 '24 19:04 sestinj

@Iamshankhadeep @sestinj Adding the if block from llmStreamChat to runNodeJsSlashCommand on its own doesn't work because the abort is never triggered. I think this is because ideStreamRequest never yields anything back to _streamSlashCommand (where the state.active property is checked and abort is called) until the end of the stream.

However, if we keep the check for setInactive message ideStreamRequest's handler, then the abort is sent and the streaming cancels on command! This works.

Note that this still doesn't work for ctrl+backspace (only works for ctrl+shift+backspace).

justinmilner1 avatar Apr 01 '24 21:04 justinmilner1

@justinmilner1 yes this looks perfect! @Iamshankhadeep would you want to adjust to use this solution, or would it be most convenient if I went ahead and just changed this locally and pushed?

sestinj avatar Apr 04 '24 02:04 sestinj

@sestinj I have updated the PR, with the new code that @justinmilner1 suggested.

Iamshankhadeep avatar Apr 04 '24 06:04 Iamshankhadeep

I do think the 'shift+ctrl+backspace' shortcut should be added to package.json keybindings pathway, to match the documentation. I don't think the gui.tsx pathway can provide the cancellation without major refactoring.

justinmilner1 avatar Apr 04 '24 14:04 justinmilner1

Hey @Iamshankhadeep - Do you want to wrap this one up?

justinmilner1 avatar Apr 29 '24 21:04 justinmilner1

Okay I'm gonna get this closed out

justinmilner1 avatar May 09 '24 19:05 justinmilner1

Because of a refactor that happened I ended up having to tweak the way the "abort" message was passed, but the basic solution still held: https://github.com/continuedev/continue/commit/5bf08de42c73265230042aaf186f5bc08e5e4efd

Closing this now, but thank you for figuring out how to do it!

sestinj avatar Jun 28 '24 02:06 sestinj