Skip to content

Conversation

@renbaoshuo
Copy link
Member

@renbaoshuo renbaoshuo commented Oct 14, 2025

只是移动了代码,尚未测试,暂时置于 Draft 状态。

代码来自 https://github.com/renbaoshuo/S2OJ

@renbaoshuo renbaoshuo requested a review from Copilot October 14, 2025 15:39
@renbaoshuo renbaoshuo self-assigned this Oct 14, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the web application to replace the V8JS-based markdown renderer with Parsedown as the markdown rendering engine. This change eliminates the dependency on the V8 JavaScript engine and its PHP extension.

Key changes:

  • Removes V8JS dependency from installation and Docker setup
  • Implements a new UOJMarkdown class extending ParsedownMath with custom features
  • Updates blog editor to use Parsedown instead of V8JS with marked.js

Reviewed Changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
web/install.sh Removes V8JS installation and configuration from setup script
web/app/models/UOJMarkdown.php Adds new markdown parser class with user mentions and table styling
web/app/models/UOJBlogEditor.php Replaces V8JS markdown processing with Parsedown implementation
web/app/models/HTML.php Adds factory method for creating UOJMarkdown instances
web/Dockerfile Removes V8JS installation from Docker build process

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -0,0 +1,80 @@
<?php

require_once __DIR__ . '../vendor/parsedown/ParsedownMath.php';
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

The path concatenation is incorrect. There's a missing directory separator between __DIR__ and the relative path. It should be __DIR__ . '/../vendor/parsedown/ParsedownMath.php'.

Suggested change
require_once __DIR__ . '../vendor/parsedown/ParsedownMath.php';
require_once __DIR__ . '/../vendor/parsedown/ParsedownMath.php';

Copilot uses AI. Check for mistakes.
Comment on lines +115 to 128
$marked = function($md) use ($parsedown, $purifier) {
$dom = new DOMDocument;
$dom->loadHTML(mb_convert_encoding($parsedown->text($md), 'HTML-ENTITIES', 'UTF-8'));
$elements = $dom->getElementsByTagName('li');

foreach ($elements as $element) {
$element->setAttribute(
'class',
$element->getAttribute('class') . ' fragment'
);
}

return $purifier->purify($dom->saveHTML());
};
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

The loadHTML method may generate warnings for malformed HTML. Additionally, saveHTML() returns the entire HTML document including <html> and <body> tags, which is likely not desired. Consider using libxml_use_internal_errors(true) to suppress warnings and saveHTML($dom->documentElement) or extracting only the body content.

Copilot uses AI. Check for mistakes.
Comment on lines +121 to +123
$element->setAttribute(
'class',
$element->getAttribute('class') . ' fragment'
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

This will create a leading space in the class attribute if the element has no existing class, resulting in a class value like ' fragment'. Consider checking if the existing class is empty or trimming the result.

Suggested change
$element->setAttribute(
'class',
$element->getAttribute('class') . ' fragment'
$current_class = $element->getAttribute('class');
$element->setAttribute(
'class',
($current_class === '' ? 'fragment' : $current_class . ' fragment')

Copilot uses AI. Check for mistakes.
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