Add copyWith and concatenate methods to ChatMessage#760
Add copyWith and concatenate methods to ChatMessage#760polina-c merged 17 commits intoflutter:mainfrom
copyWith and concatenate methods to ChatMessage#760Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces copyWith and concatenate methods, along with a + operator, to the ChatMessage class. The changes are well-implemented and include thorough tests, following standard Dart practices. My review identified a minor inaccuracy in the documentation for the + operator concerning how role and finishStatus are handled during concatenation. I've provided a suggestion to make these comments more precise and align them with the code's behavior.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
copyWith and concatenate methods and + operator to ChatMessagecopyWith and concatenate methods to ChatMessage
| /// - The finish status of the first message is used if it's not null; | ||
| /// otherwise, the second message's finish status is used. | ||
| /// - If metadata keys are the same, the first message's metadata is used. | ||
| ChatMessage concatenate(ChatMessage other, {bool throwOnConflicts = true}) { |
There was a problem hiding this comment.
I wonder if it would be better to just call this concatenateParts and just supply it with List<Part> to avoid the complications of combining the other fields of ChatMessage with this message.
Or maybe just remove throwOnConflicts entirely and make it always throw of these things don't combine well. The non-throwing version just feels like it will cause more problems than it will help.
There was a problem hiding this comment.
yes, removed throwOnConflicts, thanks for good suggestion
| @@ -189,7 +189,7 @@ final class ChatMessage { | |||
| return ChatMessage( | |||
There was a problem hiding this comment.
This could use copyWith and only copy parts and status.
No description provided.