Skip to content
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

Fix response generation broken by themeable syntax highlighting #2494

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

manyoso
Copy link
Collaborator

@manyoso manyoso commented Jul 1, 2024

🚀 This description was created by Ellipsis for commit 8b1fbfb

Summary:

Fixes response generation issue by adding forceUpdate method in ChatModel and invoking it in ChatView.qml to handle theme changes.

Key points:

  • Added forceUpdate method in gpt4all-chat/chatmodel.h to trigger dataChanged signal for ValueRole.
  • Modified gpt4all-chat/qml/ChatView.qml to call chatModel.forceUpdate(index) for reprocessing text when theme changes.

Generated with ❤️ by ellipsis.dev

…ting.

Signed-off-by: Adam Treat <treat.adam@gmail.com>
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to 8b1fbfb in 41 seconds

More details
  • Looked at 31 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_X959FlqLYkSg5GgV


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

3 days left in your free trial, upgrade for $20/seat/month or contact us.

@@ -966,7 +966,7 @@ Rectangle {
textProcessor.codeColors.headerColor = theme.codeHeaderColor
textProcessor.codeColors.backgroundColor = theme.codeBackgroundColor
textProcessor.textDocument = textDocument
myTextArea.text = value
chatModel.forceUpdate(index); // called to trigger a reprocessing of the text
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using forceUpdate to trigger a re-render of text when theme or font settings change might lead to unnecessary performance overhead. Consider updating only the visual properties directly affected by these changes instead of reprocessing the entire text.

@manyoso manyoso merged commit 56834a2 into main Jul 1, 2024
6 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant