Skip to content

Conversation

@D-Sketon
Copy link
Member

@D-Sketon D-Sketon commented Jan 18, 2025

What does it do?

fix #5433

Screenshots

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.
@github-actions
Copy link

How to test

git clone -b fix/comment https://github.com/D-Sketon/hexo.git
cd hexo
npm install
npm test
@coveralls
Copy link

coveralls commented Jan 18, 2025

Pull Request Test Coverage Report for Build 18786185367

Details

  • 50 of 50 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 99.528%

Totals Coverage Status
Change from base Build 18786079136: 0.002%
Covered Lines: 9919
Relevant Lines: 9966

💛 - Coveralls
data.content.should.eql([
'<p>foo</p>',
'<!--',
`<hexoPostRenderCodeBlock>${highlighted}</hexoPostRenderCodeBlock>`,

This comment was marked as off-topic.

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe we can solve it in backtick_code_block.ts

Copy link
Member

Choose a reason for hiding this comment

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

You are right. I forgot that the code block was processed before post render

@D-Sketon
Copy link
Member Author

The negative lookbehind and negative lookahead /(?<!<!--[^(?:\-\->)]*?)^((?:[^\S\r\n]*>){0,3}[^\S\r\n]*)(`{3,}|~{3,})[^\S\r\n]*((?:.*?[^`\s])?)[^\S\r\n]*\n((?:[\s\S]*?\n)?)(?:(?:[^\S\r\n]*>){0,3}[^\S\r\n]*)\2[^\S\r\n]?(\n+|$)(?![^(?:<!\-\-)]*?-->)/gm caused a performance regression 😓

@D-Sketon D-Sketon requested a review from a team February 13, 2025 12:52
data.content.should.eql([
'<p>foo</p>',
'<!--',
`<hexoPostRenderCodeBlock>${highlighted}</hexoPostRenderCodeBlock>`,
Copy link
Member

Choose a reason for hiding this comment

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

You are right. I forgot that the code block was processed before post render

Comment on lines +40 to +46
restoreComments(str: string) {
return str.replace(rCommentHolder, CodeBlockEscape.restoreContent(this.stored));
}

escapeComments(str: string) {
return str.replace(rCommentEscape, (_, content) => CodeBlockEscape.escapeContent(this.stored, 'comment', content));
}
Copy link
Member

@uiolee uiolee Feb 13, 2025

Choose a reason for hiding this comment

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

It looks a bit weird. Your code repeatedly escape and restore comments in 'post' and 'backtick_code_block'.

Is it possible to work like 'backtick_code_block', escape comment before post render(and before 'backtick_code_block'), restore them at the end of post render.(I am assume that the comments should not be rendered at all. I am not sure if it is right.)

Copy link
Member Author

Choose a reason for hiding this comment

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

This means we need to share the escaped cache on a per-post basis between before_post_render and post_render (either by mounting a stored on each post or maintaining a global map). I'm not sure if this will increase the complexity of the code, and whether holding the cache for a long time will affect garbage collection. However, regardless, I will give it a try.

@uiolee uiolee requested a review from a team February 13, 2025 13:41
@D-Sketon D-Sketon merged commit 3a36ca6 into hexojs:master Oct 25, 2025
23 checks passed
@D-Sketon D-Sketon deleted the fix/comment branch October 25, 2025 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants