feat(ChatBot): add MarkdownContent component#399
Merged
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a more specific exception type than
Exceptionin theGetMarkdownmethod's catch block. - The
HandleUnclosedThinkTagsmethod could potentially be simplified using HtmlAgilityPack.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logger.LogError(ex, "{GetMakrDown} method throw exception", nameof(GetMarkdown)); |
There was a problem hiding this comment.
suggestion (typo): Fix typo in error log placeholder.
The placeholder '{GetMakrDown}' seems to be a typo. It would be clearer and more consistent to use '{GetMarkdown}' in order to accurately reflect the method name.
Suggested change
| Logger.LogError(ex, "{GetMakrDown} method throw exception", nameof(GetMarkdown)); | |
| Logger.LogError(ex, "{GetMarkdown} method throw exception", nameof(GetMarkdown)); |
| return html; | ||
| } | ||
|
|
||
| private static string HandleUnclosedThinkTags(string content) |
There was a problem hiding this comment.
issue (complexity): Consider using an HTML parser library like HtmlAgilityPack to handle unclosed "think" tags instead of regex and string manipulation.
One way to reduce the “magic” of nested regex and manual string concatenation is to encapsulate the “think” tag repair logic into a small helper that uses a lightweight HTML parser rather than ad hoc regex-splitting. For example, you can replace your current implementation with one based on [HtmlAgilityPack](https://html-agility-pack.net/) to “repair” unclosed tags. This makes the intent explicit and the code easier to maintain.
For example, you might create a helper like this:
```csharp
using HtmlAgilityPack;
private static string FixThinkTags(string content)
{
if (string.IsNullOrEmpty(content)) return content;
// Load the content into an HTML document
var doc = new HtmlDocument();
doc.OptionFixNestedTags = true;
doc.LoadHtml(content);
// Select all nodes with the "think" tag.
// (HtmlAgilityPack will auto-close unclosed tags on load)
var thinkNodes = doc.DocumentNode.SelectNodes("//think");
if (thinkNodes != null)
{
foreach (var node in thinkNodes)
{
// Optionally, further manipulate the node here if needed...
// e.g. add a div wrapper if that was your intent when converting via regex
var div = HtmlNode.CreateNode("<div class=\"think\"></div>");
div.InnerHtml = node.InnerHtml;
node.ParentNode.ReplaceChild(div, node);
}
}
return doc.DocumentNode.OuterHtml;
}Then in your GetMarkdown method replace the call to HandleUnclosedThinkTags with:
toHtml = FixThinkTags(toHtml);This refactoring does not remove any functionality but delegates tag fixing to a dedicated parser which is easier to understand, unit test, and maintain. If you do not want to add a dependency now, consider extracting the regex/string-splitting logic into a separate method with clear inline documentation representing a “repair” step. Either way, encapsulating this complexity into a focused helper reduces the maintenance burden.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Link issues
fixes #398
Summary By Copilot
This pull request introduces a new component,
BootstrapBlazor.ChatBot, to theBootstrapBlazorproject. The changes include updates to the solution file and the addition of new project files for the chatbot component.Solution file updates:
BootstrapBlazor.ChatBotproject to the solution (BootstrapBlazor.Extensions.sln).Globalsection. [1] [2]New project files for
BootstrapBlazor.ChatBot:BootstrapBlazor.ChatBot.csprojwith necessary dependencies and metadata.MarkdownContent.razorcomponent to render markdown content and included necessary styles.MarkdownContentcomponent inMarkdownContent.razor.cs, including markdown processing and custom tag handling._Imports.razorto include necessary namespaces for the new component.Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add a new MarkdownContent component to the BootstrapBlazor.ChatBot project for rendering and processing markdown content with advanced features
New Features:
Enhancements: