-
Notifications
You must be signed in to change notification settings - Fork 7.7k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Markdown support #2476
Markdown support #2476
Conversation
Signed-off-by: Adam Treat <treat.adam@gmail.com>
Signed-off-by: Adam Treat <treat.adam@gmail.com>
There was a problem hiding this 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 320e184 in 1 minute and 6 seconds
More details
- Looked at
75
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_OgMDOApf7GpFkB33
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.
⌛ 7 days left in your free trial, upgrade for $20/seat/month or contact us.
@@ -925,6 +926,17 @@ void ResponseText::handleTextChanged() | |||
m_isProcessingText = false; | |||
} | |||
|
|||
void replaceAndInsertMarkdown(int startIndex, int endIndex, QTextDocument *doc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The replaceAndInsertMarkdown
function does not handle the case where endIndex
is greater than the document's character count, which can lead to an out-of-range error. Consider adding a check to ensure endIndex
does not exceed the document's character count before setting the cursor position.
int nonCodeStart = lastIndex; | ||
int nonCodeEnd = matchesCode[index].capturedStart(); | ||
if (nonCodeEnd > nonCodeStart) | ||
replaceAndInsertMarkdown(nonCodeStart, nonCodeEnd, doc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The replaceAndInsertMarkdown
function is called with nonCodeEnd
as the second argument, which might not include the last character of the non-code segment due to how capturedStart()
is defined. It might be necessary to adjust this by adding 1 to nonCodeEnd
to include the last character of the segment.
} | ||
|
||
if (lastIndex < doc->characterCount()) | ||
replaceAndInsertMarkdown(lastIndex, doc->characterCount() - 1, doc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The replaceAndInsertMarkdown
function is called with doc->characterCount() - 1
as the second argument, which might exclude the last character of the document. Consider using doc->characterCount()
instead to include the entire document content.
Signed-off-by: Adam Treat <treat.adam@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 3799816 in 39 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. gpt4all-chat/qml/ChatView.qml:1004
- Draft comment:
The PR description mentions an adjustment to the width oflistView.contentItem
by subtracting 15 units, but the diff provided does not show this change. Instead, it shows a change in the text of aTextArea
element. Please ensure that the intended changes are correctly committed and reflected in the PR. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_sEMqASRzdEyeFGPq
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 7 days left in your free trial, upgrade for $20/seat/month or contact us.
Signed-off-by: Adam Treat <treat.adam@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on cb8a4c4 in 1 minute and 0 seconds
More details
- Looked at
201
lines of code in8
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. gpt4all-chat/qml/Theme.qml:197
- Draft comment:
TheconversationDivider
property is directly returningdividerColor
without considering the theme settings, which might not be consistent with the intended theme-specific customization. Consider using a switch case based onMySettings.chatTheme
to return different colors for different themes, similar to other properties in this file. - Reason this comment was not posted:
Confidence of 30% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_wNZF5IgvGlVhm2Zd
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 7 days left in your free trial, upgrade for $20/seat/month or contact us.
Summary:
Added markdown support in
ResponseText
, adjusted UI element widths, and modified divider heights and colors in various QML files.Key points:
replaceAndInsertMarkdown
function ingpt4all-chat/responsetext.cpp
to handle markdown insertion.ResponseText::handleCodeBlocks
ingpt4all-chat/responsetext.cpp
to process non-code blocks as markdown.listView.contentItem
ingpt4all-chat/qml/ChatView.qml
by subtracting 15 units.gpt4all-chat/qml/ApplicationSettings.qml
,gpt4all-chat/qml/ChatDrawer.qml
,gpt4all-chat/qml/ChatView.qml
,gpt4all-chat/qml/CollectionsDrawer.qml
,gpt4all-chat/qml/LocalDocsSettings.qml
,gpt4all-chat/qml/ModelSettings.qml
,gpt4all-chat/qml/MySettingsStack.qml
) from 2 to 1 unit.conversationDivider
color ingpt4all-chat/qml/Theme.qml
to usedividerColor
.Generated with ❤️ by ellipsis.dev