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 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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

NoahDragon commented Dec 19, 2016

Could you please also add the test case for it?

Copy link

ivan-nginx left a comment

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

This comment has been minimized.

Copy link
Contributor

curbengh commented Oct 30, 2018

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

@YoshinoriN

This comment has been minimized.

Copy link
Member

YoshinoriN commented Nov 10, 2018

@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

This comment has been minimized.

Copy link
Contributor

tcrowe commented Nov 10, 2018

Good idea @YoshinoriN

@curbengh

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

YoshinoriN commented Nov 13, 2018

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

@JLHwung

This comment has been minimized.

Copy link
Collaborator

JLHwung commented Dec 1, 2018

@seaoak Please resolve the conflict.

@stevenjoezhang

This comment has been minimized.

Copy link
Contributor

stevenjoezhang commented Sep 30, 2019

Any updates on this thread?

curbengh and others added 3 commits Oct 10, 2019
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.
@seaoak

This comment has been minimized.

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 seaoak:feature/allow-backtick-code-block-in-blockquote-tag-plugin branch from bb9aaa5 to 1c58748 Oct 11, 2019
@seaoak

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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)

This comment has been minimized.

Copy link
@curbengh

curbengh Oct 12, 2019

Contributor

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

@curbengh

This comment has been minimized.

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 seaoak:feature/allow-backtick-code-block-in-blockquote-tag-plugin branch from 1c58748 to 8240f14 Oct 13, 2019
v4.0.0 automation moved this from To do to In progress Oct 13, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor

curbengh commented Oct 13, 2019

@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 hexojs/core Oct 13, 2019
@seaoak

This comment has been minimized.

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 approved these changes Oct 13, 2019
@SukkaW SukkaW merged commit 79bdc95 into hexojs:master Oct 13, 2019
4 checks passed
4 checks passed
Travis CI - Pull Request Build Passed
Details
codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.005%) to 97.278%
Details
v4.0.0 automation moved this from In progress to Done Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v4.0.0
  
Done
10 participants
You can’t perform that action at this time.