Refactor clineprovider proof of concept #2144
Conversation
…actor/clineprovider/proof_of_concept
|
- interface mismatch workaround removed - implemented async getCurrentTaskStack using provider - updated interface to reflect async behavior
bramburn
changed the title
| const taskStack = await this.getCurrentTaskStack() | ||
| if (taskStack.length > 0) { | ||
| // We need to find the first task in the stack | ||
| // This is a simplification - in a real implementation, you might need to |
There was a problem hiding this comment.
This comment confuses me. Is this not a "real implementation"?
| * Adds a new Cline instance to the stack | ||
| * @param cline The Cline instance to add | ||
| */ | ||
| async addClineToStack(cline: Cline): Promise<void> { |
There was a problem hiding this comment.
Why is this method (and all the methods here) async?
Seems like it adds unecessary complexity for the code that uses this interface. Is there a good reason the interface needs to be async? It seems to just perform synchronous array operations.
There was a problem hiding this comment.
I think you have a point on there, I will re-review these. In the original clineProvider, the methods that were calling the addClineTostack and removeCline were already using async. I would have thought this is because the cline instances are still being processed because of the calls to the LLM still running the streams.
Thank you for the feedback, I'm trying to get better at contribution and get into SWE one day. I will rebuild it and run some test
|
|
||
| async showTaskWithId(id: string) { | ||
| if (id !== this.getCurrentCline()?.taskId) { | ||
| const currentCline = await this.getCurrentCline() |
There was a problem hiding this comment.
There are loads of changes like this, which seem to be necessitated by the fact that getCurrentCline thas become async. But I don't understand what is driving that complexity.
|
At a high level, I don't really understand the motivation here. In the old code, you have the You have replaced this array with a class I'm all for removing complexity from Overall ClineProvider seems to have got bigger, not smaller, as a result of these changes, which I think reflects that these changes add complexity, but without really delivering much in the way of simplification. elsewhere. Perhaps I am missing something. Is there some future benefit to |
hannesrudolph
moved this from PR [Pre Approval Review]
to PR [Draft/WIP]
in Roo Code Roadmap
|
Hey @bramburn, |
github-project-automation
bot
moved this from PR [Draft/WIP]
to Done
in Roo Code Roadmap
3 participants
Context
A proof of concept of decoupling clineprovider file. It is a first in a series of additional changes in the code to decouple it to better manage the code. The next one would be dealing with the webview, and states.
Implementation
Screenshots
How to Test
Get in Touch