chat: fix diff not showing for streaming chat pills #241799
Merged
+40
−19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The addition of `typesToSkip` broke because the chat pill looks back for the last undo stop to know what to diff between. See: microsoft/vscode-copilot#13229 (comment) This gets rid of that, and instead allows IChatContentPart to choose not to render a domNode. We can then 'render' undo stops without side-effects and fix the issue. This is kind of special but I feel like this is the cleanest way to do things now, and I can imagine other parts might use this in the future. Fixes microsoft/vscode-copilot#13229
connor4312
commented
Feb 25, 2025
| } | ||
|
|
||
| // todo@connor4312/roblourens: should this throw or renderNoContent to avoid progressive render thrashing? |
There was a problem hiding this comment.
@roblourens what do you think?
There was a problem hiding this comment.
Oh, sorry for the break! If this method returns undefined, it tries to render that part forever? Your renderNoContent makes sense. Actually there's a couple cases already that render an empty dom element, like when a progress message hides.
justschen
approved these changes
Feb 25, 2025
roblourens
approved these changes
Feb 25, 2025
| } | ||
|
|
||
| // todo@connor4312/roblourens: should this throw or renderNoContent to avoid progressive render thrashing? |
There was a problem hiding this comment.
Oh, sorry for the break! If this method returns undefined, it tries to render that part forever? Your renderNoContent makes sense. Actually there's a couple cases already that render an empty dom element, like when a progress message hides.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The addition of
typesToSkipbroke because the code pill looks back forthe last undo stop to know what to diff between. See:
https://github.com/microsoft/vscode-copilot/issues/13229#issuecomment-2679994707
This gets rid of that, and instead allows IChatContentPart to choose
not to render a domNode. We can then 'render' undo stops without
side-effects and fix the issue. This is kind of special but I feel like
this is the cleanest way to do things now, and I can imagine other parts
might use this in the future.
Fixes https://github.com/microsoft/vscode-copilot/issues/13229