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

chore: update to hexo-util@1.0.1 and hexo-renderer-marked@2.0.0 #3646

Merged
merged 9 commits into from Aug 30, 2019

Conversation

@tomap
Copy link
Contributor

tomap commented Aug 3, 2019

I updated both packages to RC versions, and there are a few tests to investigate after the update

@coveralls

This comment has been minimized.

Copy link

coveralls commented Aug 3, 2019

Coverage Status

Coverage increased (+0.07%) to 97.223% when pulling dced2ac on tomap:feature/PackageRC into 9b87047 on hexojs:master.

@curbengh

This comment has been minimized.

Copy link
Contributor

curbengh commented Aug 4, 2019

I can confirm the issue is caused by hexo-util@1.0.0-rc1 alone.

hexo-util@1.0.0-rc1 vs hexo-util@0.6.3:

41c86,88
<     "bluebird": "^3.4.0",
---
> 
>     "bluebird": "^3.5.2",
> 
43,46c90,104
<     "cross-spawn": "^4.0.0",
<     "highlight.js": "^9.4.0",
<     "html-entities": "^1.2.0",
<     "striptags": "^2.1.1"
---
> 
>     "cross-spawn": "^6.0.5",
> 
>     "highlight.js": "^9.13.1",
> 
>     "html-entities": "^1.2.1",
> 
>     "striptags": "^3.1.1"
> 

Possible causes: cross-spawn, highlight.js and striptags. I'm testing each dep.


Issue not resolved after downgrading deps.


Looking through the hexo-util's history, the failure starts from this commit (hexojs/hexo-util@38933b6#diff-e8acc63b1e238f3255c900eed37254b8). The commit allows empty attribute to retain (e.g. class="").

Edit: Submitted fix PR (#3648). Merged into this PR instead.

Still looking for cause of weird line numbering.


Line numbering issue is caused by hexojs/hexo-util@ed5cb6e#diff-e8acc63b1e238f3255c900eed37254b8
@segayuu


https://github.com/hexojs/hexo-util/blob/ed5cb6ed7fa1391ec12af96092d2c752f5624fc2/lib/highlight.js#L39

numbers += `<span class="line">${firstLine + i}</span><br>`;

Somehow firstLine is treated as a string, thus causing string concatenation, instead of being computed.

Submitted fix PR (hexojs/hexo-util#67).

@curbengh

This comment has been minimized.

Copy link
Contributor

curbengh commented Aug 8, 2019

There is something wrong with this PR/branch,

package.json
"hexo": "tomap/hexo"

works,

but not

"hexo": "tomap/hexo#feature/PackageRC
$ hexo -v
ERROR Local hexo not found in ~/hexo
ERROR Try running: 'npm install hexo --save'

Caused by hexo-util@1.0.0-rc2, no issue after downgraded to hexo-util@1.0.0-rc1. Probably related to hexojs/hexo-util#57 (comment).


PR fix hexojs/hexo-util#69

@curbengh

This comment has been minimized.

Copy link
Contributor

curbengh commented Aug 10, 2019

Somehow hexo-util is still outdated. Testing...
hexo-renderer-marked probably needs to be updated.


Pass 🎊

@curbengh

This comment has been minimized.

Copy link
Contributor

curbengh commented Aug 19, 2019

@curbengh

This comment has been minimized.

Copy link
Contributor

curbengh commented Aug 21, 2019

@tomap tomap mentioned this pull request Aug 21, 2019
package.json Outdated Show resolved Hide resolved
curbengh added 2 commits Aug 22, 2019
@curbengh curbengh changed the title WIP: Test hexo-util and hexo-renderer-marked RC chore: update hexo-util and hexo-renderer-marked Aug 24, 2019
@curbengh curbengh changed the title chore: update hexo-util and hexo-renderer-marked chore: update to hexo-util@1.0.0 and hexo-renderer-marked@2.0.0 Aug 24, 2019
@curbengh curbengh requested a review from hexojs/core Aug 24, 2019
@curbengh curbengh requested a review from hexojs/core Aug 25, 2019
Copy link
Member

YoshinoriN left a comment

Tag cloud color class should migrate to hexo-util.

@curbengh

This comment has been minimized.

Copy link
Contributor

curbengh commented Aug 29, 2019

eea2134 fixes #3656, but pending hexojs/hexo-util#75

hexojs/hexo-util#75 merged, pending to be published hexojs/hexo-util#76.

Published https://www.npmjs.com/package/hexo-util/v/1.0.1 to include hexojs/hexo-util#75

@YoshinoriN I think it's ready.

@curbengh curbengh changed the title chore: update to hexo-util@1.0.0 and hexo-renderer-marked@2.0.0 WIP chore: update to hexo-util@1.0.0 and hexo-renderer-marked@2.0.0 Aug 29, 2019
* to include hexojs/hexo-util#75
@curbengh curbengh requested a review from YoshinoriN Aug 29, 2019
@curbengh curbengh changed the title WIP chore: update to hexo-util@1.0.0 and hexo-renderer-marked@2.0.0 chore: update to hexo-util@1.0.1 and hexo-renderer-marked@2.0.0 Aug 29, 2019
@curbengh curbengh requested a review from hexojs/core Aug 30, 2019
Copy link
Member

YoshinoriN left a comment

@curbengh
Sorry so late. LGTM

@YoshinoriN YoshinoriN merged commit 5f05653 into hexojs:master Aug 30, 2019
4 checks passed
4 checks passed
Travis CI - Pull Request Build Passed
Details
codeclimate 7 fixed issues
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.07%) to 97.223%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.