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

Language highlight with diff on markdown in articles and books #104

Merged
merged 4 commits into from
Jan 24, 2021

Conversation

kkiyama117
Copy link
Contributor

@kkiyama117 kkiyama117 commented Jan 20, 2021

About

Resolves #86 .
Use the similar way with diff-highlight plugin of markdown-it-prism.

Links

https://zenn.dev/kkiyama117/scraps/05094a5e9f8db8

Others

may conflict with #103. this change do not cause confliction unless you treat prism through markdown-it instance.

@kkiyama117
Copy link
Contributor Author

kkiyama117 commented Jan 20, 2021

TODO

@kkiyama117 kkiyama117 changed the title Feature/language diff Use language highlight with diff on markdown in articles and books Jan 20, 2021
@kkiyama117 kkiyama117 changed the title Use language highlight with diff on markdown in articles and books Language highlight with diff on markdown in articles and books Jan 20, 2021
@catnose99
Copy link
Contributor

@kkiyama117
Thanks always! I'll review it later.

Copy link
Contributor

@steelydylan steelydylan left a comment

Choose a reason for hiding this comment

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

@kkiyama117

PRいただきありがとうございます。
挙動的に問題なさそうです。

ただ今後、zenn-markdown-htmlをブラウザーもしくはWebWorker上でも動かす予定がありそのために別PRで以下の修正を予定しています。

  • zenn-editorにdiff-highlightを含めず、別モジュール化
  • diff componentを初めからロードしておく様に修正

#107

@@ -33,7 +33,7 @@
"markdown-it-inline-comments": "^1.0.1",
"markdown-it-link-attributes": "^3.0.0",
"markdown-it-task-lists": "^2.1.1",
"prismjs": "^1.22.0"
"prismjs": "^1.23.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: prismjsのマイナーバージョンの更新理由を教えていただけないでしょうか?

Copy link
Contributor Author

@kkiyama117 kkiyama117 Jan 23, 2021

Choose a reason for hiding this comment

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

A: はじめ #86 の解決に PrismJS/prism#2580 が必要かと思って試行錯誤していた時のcommitが混ざってしまってます...... 念の為手元でRevertしても特に問題なく動作しているように思われます.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kkiyama117 あ、なるほどですね!もし問題がありそうなら元に戻す可能性はありますが、今のところ大丈夫です!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revert commit を上げた方がよろしいでしょうか?

Copy link
Contributor

Choose a reason for hiding this comment

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

あ、特に大丈夫ですよ!理由が気になっただけなので!
ありがとうございます。

@catnose99 catnose99 merged commit 3965a44 into zenn-dev:master Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

プログラミング言語ごとのdiff
3 participants