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.

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

Allow backtick code block in "blockquote" tag plugin (#2318) #2321

Conversation

seaoak
Copy link
Member

@seaoak seaoak commented Dec 19, 2016

When backtick code block(s) exist as contents of a "blockquote" tag plugin,
each code block is translated to a string "undefined" in HTML (Issue #2318).

In analyzing markdown source text, while the replacement of these elements
with placeholders are nesting, recoveries from placeholders are executed
only once. So I modify to repeat the recovery process until all
placeholders are recovered.

NOTE: From Pull Request #2319, I correct the condition of exception (accept empty string as escape content).

@coveralls
Copy link

coveralls commented Dec 19, 2016

Coverage Status

Coverage increased (+0.005%) to 97.278% when pulling 11c032b on seaoak:feature/allow-backtick-code-block-in-blockquote-tag-plugin into bd70086 on hexojs:master.

@NoahDragon
Copy link
Member

Could you please also add the test case for it?

@stale stale bot added the wontfix This will not be worked on label Sep 27, 2017
@hexojs hexojs deleted a comment from stale bot Sep 27, 2017
@NoahDragon NoahDragon added enhancement New feature or request and removed wontfix This will not be worked on labels Sep 27, 2017
ivan-nginx
ivan-nginx previously approved these changes Jun 13, 2018
Copy link

@ivan-nginx ivan-nginx left a comment

Choose a reason for hiding this comment

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

BTW, all tags which have ends: true parameter not working correctly in each other. For example, codeblock also won't work in note (bscallout) tag.

This pull usefull and non merged from 19 Dec 2016. Is this +0.008% coverage so important to freeze bug for 1.5 years?

@curbengh
Copy link
Contributor

curbengh commented Oct 30, 2018

@seaoak post.js has significantly changed, please rebase.

@yoshinorin
Copy link
Member

@weyusi @NoahDragon @hexojs/core
I tried to rebase this PR to master.
But, current post.js source code changed drastic compare with this PR.

I propose we re-create other PR instead of this PR if @seaoak will not react our mention.

@tcrowe
Copy link
Contributor

tcrowe commented Nov 10, 2018

Good idea @yoshinorin

@curbengh
Copy link
Contributor

curbengh commented Nov 12, 2018

@yoshinorin
if someone can port this PR to the current post.js that would be great. Before I ping @seaoak, I also tried to re-base, but too bad it's beyond me.

I think the resursion function could fix #3318 #2821

@seaoak
Copy link
Member Author

seaoak commented Nov 12, 2018

Sorry, I'm too late to response you.
I thank you for picking up this PR!

I was busy over the past year, but quite recently I have a little time to enjoy programming.
If allowed, I'll try to rebase this PR.
Of course, I should catch up with current codes of Hexo.
So it may need about a month or more...

If anymone takes this PR, I'm pleased to leave this PR. 😄

@yoshinorin
Copy link
Member

Welcome back seaoka !!
It’s okay. Please don't worry, we patiently waiting for your PR 😄

@JLHwung
Copy link
Collaborator

JLHwung commented Dec 1, 2018

@seaoak Please resolve the conflict.

@stevenjoezhang
Copy link
Member

Any updates on this thread?

seaoak and others added 3 commits October 10, 2019 05:36
When backtick code block(s) exist as contents of a "blockquote" tag plugin,
each code block is translated to a string "undefined" in HTML (Issue hexojs#2318).

In analyzing markdown source text, while the replacement of these elements
with placeholders are nesting, recoveries from placeholders are executed
only once.  So I modify to repeat the recovery process until all
placeholders are recovered.
@seaoak
Copy link
Member Author

seaoak commented Oct 10, 2019

I'm sorry I restarted to catch up latest Hexo last month.
I had left from Hexo development for a long time,
it will take a while.

I'll be glad if you give me a chance to update this patch.

@seaoak seaoak force-pushed the feature/allow-backtick-code-block-in-blockquote-tag-plugin branch from bb9aaa5 to 1c58748 Compare October 11, 2019 20:17
@seaoak
Copy link
Member Author

seaoak commented Oct 11, 2019

I rewrite this patch for latest Hexo.
And also add a test case for this patch.

@hexojs/core Could you please review?

By the way #3318 is not fixed by this patch.
It seems like #2969.

NOTE:
I did "forced push" to this branch because the code base of Hexo changed drastically.
If it violates the common manner, I will make new PR (and close this PR).

@seaoak
Copy link
Member Author

seaoak commented Oct 12, 2019

Travis CI erros only on Node.js 6.x.
The Build options of this PR might be too old.

May I adjust build options of this PR? (Although I'm not familiar with Travis CI...)

@SukkaW
Copy link
Member

SukkaW commented Oct 12, 2019

@sseaoak We have already drop node 6 for Travis CI in #3598

I will trigger a rebuild for you.

@curbengh
Copy link
Contributor

curbengh commented Oct 12, 2019

NOTE:
I did "forced push" to this branch because the code base of Hexo changed drastically.
If it violates the common manner, I will make new PR (and close this PR).

It's fine to rebase to the latest master (in fact I often do it), which would require forced push.

Rebasing is necessary in this PR resolve the CI issue.

@@ -717,4 +717,27 @@ describe('Post', () => {
});
});

// test for Issue [#2318](https://github.com/hexojs/hexo/issues/2318)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think // test for Issue #2318 is sufficient without the full link.

@curbengh
Copy link
Contributor

curbengh commented Oct 12, 2019

By the way #3318 is not fixed by this patch.
It seems like #2969.

I created a fix hexojs/hexo-util#110 for those issues. Tested with this PR.

But the fix comes with some caveat; oh well, I would suggest user to use {% blockquote %} then.

@curbengh curbengh added this to In progress in v4.0.0 via automation Oct 12, 2019
@curbengh curbengh added this to the v4.0.0 milestone Oct 12, 2019
@curbengh curbengh moved this from In progress to To do in v4.0.0 Oct 12, 2019
test/scripts/hexo/post.js Outdated Show resolved Hide resolved
@curbengh curbengh force-pushed the feature/allow-backtick-code-block-in-blockquote-tag-plugin branch from 1c58748 to 8240f14 Compare October 13, 2019 00:50
v4.0.0 automation moved this from To do to In progress Oct 13, 2019
@curbengh
Copy link
Contributor

@seaoak

I've rebased this PR, so all test pass now. My apology if this come off as a bit rude, I just want to include this PR into v4.

@curbengh curbengh requested a review from a team October 13, 2019 00:57
@seaoak
Copy link
Member Author

seaoak commented Oct 13, 2019

@curbengh Thank you for your review and rebase! 😄
I agree all your comments and changes.
And I'm glad this patch will be included into v4.

@SukkaW SukkaW merged commit 79bdc95 into hexojs:master Oct 13, 2019
v4.0.0 automation moved this from In progress to Done Oct 13, 2019
thom4parisot pushed a commit to thom4parisot/hexo that referenced this pull request Jan 17, 2020
hexojs#2321)

* test(backtick): update comment

* Allow backtick code block in "blockquote" tag plugin (hexojs#2318)

When backtick code block(s) exist as contents of a "blockquote" tag plugin,
each code block is translated to a string "undefined" in HTML (Issue hexojs#2318).

In analyzing markdown source text, while the replacement of these elements
with placeholders are nesting, recoveries from placeholders are executed
only once.  So I modify to repeat the recovery process until all
placeholders are recovered.

* require assert
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
v4.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

10 participants