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.

Sign up for GitHub

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

refactor(post/tag): render tag before content #4171

Closed
wants to merge 6 commits into from
Closed

refactor(post/tag): render tag before content #4171

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2ef9c9b
refactor(post): render tag before content
SukkaW Mar 4, 2020
32d5638
test(post): add cases for issue #3346
SukkaW Mar 4, 2020
54d45e3
refactor(post): simplify the <escape> regex pattern
SukkaW Mar 4, 2020
1b0819a
refactor(tag): add extra blankline in tag.run()
SukkaW Mar 5, 2020
1bc7e78
refactor(tag): remove escape
SukkaW Mar 6, 2020
3bdf42d
fix(tag): simplify error handle
SukkaW Mar 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions lib/extend/tag.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ const stripIndent = require('strip-indent');
const { cyan } = require('chalk');
const nunjucks = require('nunjucks');
const Promise = require('bluebird');
const placeholder = '\uFFFC';
const rPlaceholder = /(?:<|&lt;)!--\uFFFC(\d+)--(?:>|&gt;)/g;

class NunjucksTag {
constructor(name, fn) {
Expand Down Expand Up @@ -77,7 +75,7 @@ class NunjucksBlock extends NunjucksTag {
}

run(context, args, body) {
return this._run(context, args, trimBody(body));
return this._run(context, args, trimBody(body)) + '\n';
Copy link
Member

Choose a reason for hiding this comment

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

Why add \n?

Look at the following test cases, which one is expected?

    const str = [
      'begin',
      '{% test foo bar %}',
      'test content',
      '{% endtest %}',
      'end'
    ].join('\n');
    const result = await tag.render(str);

    // there is only one `\n` before and after
    result.should.eql('begin\nfoo bar test content\nend');
    // Or should there be two `\n` at the end?
    result.should.eql('begin\nfoo bar test content\n\nend');

Copy link
Member

Choose a reason for hiding this comment

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

This extra \n is to separete a Nunjucks tag from immediate-after content.
So I expect two \n at the end.

Please refer the above comment #4171 (comment)

}
}

Expand Down Expand Up @@ -111,7 +109,7 @@ class NunjucksAsyncBlock extends NunjucksBlock {
body = () => result || '';

this._run(context, args, trimBody(body)).then(result => {
callback(err, result);
callback(err, result && `${result}\n`);
});
});
}
Expand Down Expand Up @@ -228,15 +226,8 @@ class Tag {
options = {};
}

const cache = [];

const escapeContent = str => `<!--${placeholder}${cache.push(str) - 1}-->`;

str = str.replace(/<pre><code.*>[\s\S]*?<\/code><\/pre>/gm, escapeContent);
Comment on lines -231 to -235
Copy link
Member Author

@SukkaW SukkaW Mar 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hexo only uses tag.render in post.js, which is after backtick_code_filter is executed.

In backtick_code_filter, { & } are already escape, so it is fine to render with nunjucks directly.


return Promise.fromCallback(cb => { this.env.renderString(str, options, cb); })
.catch(err => Promise.reject(formatNunjucksError(err, str)))
.then(result => result.replace(rPlaceholder, (_, index) => cache[index]));
.catch(err => Promise.reject(formatNunjucksError(err, str)));
}
}

Expand Down
89 changes: 20 additions & 69 deletions lib/hexo/post.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';

const assert = require('assert');
const moment = require('moment');
const Promise = require('bluebird');
const { join, extname } = require('path');
Expand All @@ -12,47 +11,6 @@ const yfm = require('hexo-front-matter');

const preservedKeys = ['title', 'slug', 'path', 'layout', 'date', 'content'];

const _escapeContent = (cache, str) => {
const placeholder = '\uFFFC';
return `<!--${placeholder}${cache.push(str) - 1}-->`;
};

class PostRenderCache {
constructor() {
this.cache = [];
}

escapeContent(str) {
const rEscapeContent = /<escape(?:[^>]*)>([\s\S]*?)<\/escape>/g;
return str.replace(rEscapeContent, (_, content) => _escapeContent(this.cache, content));
}

loadContent(str) {
const rPlaceholder = /(?:<|&lt;)!--\uFFFC(\d+)--(?:>|&gt;)/g;
const restored = str.replace(rPlaceholder, (_, index) => {
assert(this.cache[index]);
const value = this.cache[index];
this.cache[index] = null;
return value;
});
if (restored === str) return restored;
return this.loadContent(restored); // self-recursive for nexted escaping
}

escapeAllSwigTags(str) {
const rSwigVar = /\{\{[\s\S]*?\}\}/g;
const rSwigComment = /\{#[\s\S]*?#\}/g;
const rSwigBlock = /\{%[\s\S]*?%\}/g;
const rSwigFullBlock = /\{% *(.+?)(?: *| +.*?)%\}[\s\S]+?\{% *end\1 *%\}/g;

const escape = _str => _escapeContent(this.cache, _str);
return str.replace(rSwigFullBlock, escape)
.replace(rSwigBlock, escape)
.replace(rSwigComment, '')
.replace(rSwigVar, escape);
}
}

const prepareFrontMatter = data => {
for (const [key, item] of Object.entries(data)) {
if (moment.isMoment(item)) {
Expand Down Expand Up @@ -96,7 +54,7 @@ class Post {
const ctx = this.context;
const { config } = ctx;

data.slug = slugize((data.slug || data.title).toString(), {transform: config.filename_case});
data.slug = slugize((data.slug || data.title).toString(), { transform: config.filename_case });
data.layout = (data.layout || config.default_layout).toLowerCase();
data.date = data.date ? moment(data.date) : moment();

Expand Down Expand Up @@ -135,7 +93,7 @@ class Post {
let yfmSplit;

return this._getScaffold(data.layout).then(scaffold => {
const frontMatter = prepareFrontMatter({...data});
const frontMatter = prepareFrontMatter({ ...data });
yfmSplit = yfm.split(scaffold);

return tag.render(yfmSplit.data, frontMatter);
Expand Down Expand Up @@ -183,7 +141,7 @@ class Post {
const ctx = this.context;
const { config } = ctx;
const draftDir = join(ctx.source_dir, '_drafts');
const slug = slugize(data.slug.toString(), {transform: config.filename_case});
const slug = slugize(data.slug.toString(), { transform: config.filename_case });
data.slug = slug;
const regex = new RegExp(`^${escapeRegExp(slug)}(?:[^\\/\\\\]+)`);
let src = '';
Expand Down Expand Up @@ -248,52 +206,45 @@ class Post {
// disable Nunjucks when the renderer spcify that.
const disableNunjucks = ext && ctx.render.renderer.get(ext) && !!ctx.render.renderer.get(ext).disableNunjucks;

const cacheObj = new PostRenderCache();

return promise.then(content => {
data.content = content;

// Run "before_post_render" filters
return ctx.execFilter('before_post_render', data, {context: ctx});
return ctx.execFilter('before_post_render', data, { context: ctx });
}).then(() => {
data.content = cacheObj.escapeContent(data.content);

if (isSwig) {
// Render with Nunjucks if this is a swig file
// Render with Nunjucks if the post is a swig file
return tag.render(data.content, data);
}
stevenjoezhang marked this conversation as resolved.
Show resolved Hide resolved

// Escape all Swig tags
if (!disableNunjucks) {
data.content = cacheObj.escapeAllSwigTags(data.content);
}

const options = data.markdown || {};
if (!config.highlight.enable) options.highlight = null;

ctx.log.debug('Rendering post: %s', magenta(source));
// Render with markdown or other renderer
return ctx.render.render({
text: data.content,

const promise = text => ctx.render.render({
text,
path: source,
engine: data.engine,
toString: true,
onRenderEnd(content) {
// Replace cache data with real contents
data.content = cacheObj.loadContent(content);

// Return content after replace the placeholders
if (disableNunjucks) return data.content;

// Render with Nunjucks
return tag.render(data.content, data);
data.content = content.replace(/<!--hexoPostRenderEscape:/g, '')
.replace(/:hexoPostRenderEscape-->/g, '');
return data.content;
}
}, options);

if (!disableNunjucks) {
return tag.render(data.content, data).then(promise);
}

// Nunjucks is disabled
return promise(data.content);

}).then(content => {
data.content = content;

// Run "after_post_render" filters
return ctx.execFilter('after_post_render', data, {context: ctx});
return ctx.execFilter('after_post_render', data, { context: ctx });
}).asCallback(callback);
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/plugins/filter/before_post_render/backtick_code_block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function backtickCodeBlock(data) {
.replace(/{/g, '&#123;')
.replace(/}/g, '&#125;');

return `${start}<escape>${content}</escape>${end}`;
return `${start}<!--hexoPostRenderEscape:${content}:hexoPostRenderEscape-->${end}`;
});
}

Expand Down
28 changes: 25 additions & 3 deletions test/fixtures/post_render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const code = [
' sleep()'
].join('\n');

const content = [
exports.content = [
'# Title',
'``` python',
code,
Expand All @@ -24,7 +24,19 @@ const content = [
'{% endquote %}'
].join('\n');

exports.content = content;
exports.content_for_issue_3346 = [
'# Title',
'```',
'{% test1 %}',
'{{ test2 }}',
'```',
'some content',
'',
'## Another title',
'{% blockquote %}',
'quote content',
'{% endblockquote %}'
].join('\n');

exports.expected = [
'<h1 id="Title"><a href="#Title" class="headerlink" title="Title"></a>Title</h1>',
Expand All @@ -33,7 +45,7 @@ exports.expected = [
'<h2 id="Another-title"><a href="#Another-title" class="headerlink" title="Another title"></a>Another title</h2>',
'<blockquote>',
'<p>quote content</p>\n',
'</blockquote>\n\n',
'</blockquote>\n\n\n',
'<blockquote><p>quote content</p>\n',
'<footer><strong>Hello World</strong></footer></blockquote>'
].join('');
Expand All @@ -50,3 +62,13 @@ exports.expected_disable_nunjucks = [
'quote content<br>',
'{% endquote %}</p>'
].join('');

exports.expected_for_issue_3346 = [
'<h1 id="Title"><a href="#Title" class="headerlink" title="Title"></a>Title</h1>',
highlight('{% test1 %}\n{{ test2 }}').replace(/{/g, '&#123;').replace(/}/g, '&#125;'), // Escaped by backtick_code_block
'\n<p>some content</p>\n',
'<h2 id="Another-title"><a href="#Another-title" class="headerlink" title="Another title"></a>Another title</h2>',
'<blockquote>',
'<p>quote content</p>\n',
'</blockquote>'
].join('');
6 changes: 3 additions & 3 deletions test/scripts/extend/tag.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('Tag', () => {
].join('\n');

const result = await tag.render(str);
result.should.eql('foo bar test content');
result.should.eql('foo bar test content\n');
});

it('register() - async block', async () => {
Expand All @@ -52,7 +52,7 @@ describe('Tag', () => {
].join('\n');

const result = await tag.render(str);
result.should.eql('foo bar test content');
result.should.eql('foo bar test content\n');
});

it('register() - nested test', async () => {
Expand Down Expand Up @@ -111,7 +111,7 @@ describe('Tag', () => {
].join('\n');

const result = await tag.render(str);
result.should.eql('test content');
result.should.eql('test content\n');
});

it('register() - async callback', async () => {
Expand Down
Loading