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

Fixes for issue #2519. #2543

Merged
merged 5 commits into from
Jul 8, 2024
Merged

Fixes for issue #2519. #2543

merged 5 commits into from
Jul 8, 2024

Conversation

manyoso
Copy link
Collaborator

@manyoso manyoso commented Jul 3, 2024

Send text directly into the text document from the chat view rather than using a signal to post process the text. This is still not ideal in that we want to move to preprocessing the text in the future and to do so by streaming the content in rather than replacing it all at the same time, but it does fix a hang that we experience with an infinite recursive loop of text changed signals in the markdown processing.

This fixes issue #2519.


🚀 This description was created by Ellipsis for commit 124d8be

Summary:

Fixes issue #2519 by sending text directly to the text document in the chat view, avoiding infinite recursive loops during markdown processing.

Key points:

  • Fixes issue Chat gets stuck processing markdown with certain prompts in 3.0.0 #2519 by sending text directly to the text document in the chat view, avoiding infinite recursive loops during markdown processing.
  • Removed forceUpdate method from gpt4all-chat/chatmodel.h.
  • Added valueChanged signal in ChatModel class in gpt4all-chat/chatmodel.h.
  • Modified updateValue method in ChatModel to emit valueChanged signal.
  • Replaced m_textDocument with m_quickTextDocument in ChatViewTextProcessor class in gpt4all-chat/chatviewtextprocessor.cpp and gpt4all-chat/chatviewtextprocessor.h.
  • Added setValue method to ChatViewTextProcessor class to set plain text directly.
  • Updated gpt4all-chat/qml/ChatView.qml to use textProcessor.setValue(value) instead of chatModel.forceUpdate(index).

Generated with ❤️ by ellipsis.dev

@manyoso manyoso requested a review from cebtenzzre July 3, 2024 17:07
using a signal to post process the text. This is still not ideal in that
we want to move to preprocessing the text in the future and to do so by
streaming the content in rather than replacing it all at the same time,
but it does fix a hang that we experience with an infinite recursive loop
of text changed signals in the markdown processing.

This fixes issue #2519.

Signed-off-by: Adam Treat <[email protected]>
Copy link
Member

@cebtenzzre cebtenzzre left a comment

Choose a reason for hiding this comment

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

This change breaks the enable/disable markdown button—it cannot be enabled again once disabled.

@manyoso
Copy link
Collaborator Author

manyoso commented Jul 8, 2024

This change breaks the enable/disable markdown button—it cannot be enabled again once disabled.

Should be fixed now.

@cebtenzzre cebtenzzre merged commit c11e0f4 into main Jul 8, 2024
6 of 12 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.

2 participants