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

Update markdown-it version and relevant patches #1701

Merged
merged 3 commits into from
Jan 14, 2022

Conversation

tlylt
Copy link
Contributor

@tlylt tlylt commented Dec 31, 2021

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Overview of changes:
Fixes #1676. Bug example in #1675 (merging this will hence fixes #1675).
The <hr> multiple class issue is caused by the markdown-it-attrs plugin that we are using. I have opened a PR to fix it in the upstream repo and it has been merged. Hence, I am updating the markdown-it-attrs and markdown-it version so that the bug fix can be applied to markbind.

(edited)
The Markdown-it changelog mentions a few updates which may affect some of the patches that we keep
in the packages/core/src/lib/markdown-it folder. I have updated two instances where the changes need
to be forwarded. The change will help closes #1708 and closes #1709 .

Anything you'd like to highlight / discuss:
Both packages have an increase in the first digit of the version number (Major releases that have changes that break backward compatibility). I looked through their changelogs here and here and didn't see any obvious concerns. I also ran the test cases locally and everything looks fine with the update. However, I am not sure if there are other things that I should look out for. Any advice would be appreciated.

Testing instructions:
Manually, one can create a test site and put in index.md the following:

--- {.a .b}

The resulted HTML would previously look like this:

<hr class="b">

and now it should look like this:

<hr class="a b">

Note: I will be adding functional tests for hr elements in another pending PR once this is merged.

Proposed commit message: (wrap lines at 72 characters)
A minor bug in markdown-it-attrs prevents the addition
of multiple classes for the hr elements. Its latest version
fixes the issue.

Let's update the version of the plugin and the core
markdown-it package to fix the above-mentioned bug.


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Minor bug in markdown-it-attrs prevents addition
of multiple classes for <hr> elements. The
latest version fixes the issue.

Let's update the version of the plugin and the
core markdown-it package to fix the above
mentioned bug.
@ang-zeyu
Copy link
Contributor

ang-zeyu commented Jan 3, 2022

Nice work upstream!

However, I am not sure if there are other things that I should look out for. Any advice would be appreciated.

We made a couple of patches for markdown-it's default rules here to allow custom component names to work (similar to what vuepress has done here), and to add support for parsing other tags like script/style tags (the "specialTags" option here).

Are there any changes that should be forwarded to these files? e.g. this one

@tlylt
Copy link
Contributor Author

tlylt commented Jan 3, 2022

We made a couple of patches for markdown-it's default rules here to allow custom component names to work (similar to what vuepress has done here), and to add support for parsing other tags like script/style tags (the "specialTags" option here).

Yes I think I'll need to check through these to ensure all changes are forwarded (including those performance updates that may not affect any syntax)

Are there any changes that should be forwarded to these files? e.g. this one

Yup, It seems like I will need to add textarea.

I also found one more possible change that I may need to make (meta elements are considered inline now).
Will update this PR once I completed the relevant changes.

@tlylt tlylt changed the title Update markdown-it and its plugin version [WIP] Update markdown-it and its plugin version Jan 3, 2022
@tlylt tlylt mentioned this pull request Jan 13, 2022
9 tasks
@tlylt tlylt changed the title [WIP] Update markdown-it and its plugin version [WIP] Update markdown-it version and relevant patches Jan 13, 2022
The Markdown-it changelog mentions a few updates
which may affect some of the patches that we keep
in the packages/core/src/lib/markdown-it folder.

Let's update some of the content to match the
latest changes.

The markdown-it-attrs package has to be updated
together due to it's peer dependency requirment
on markdown-it. The update on the plugin is also
required to fix a reported bug.
@tlylt tlylt changed the title [WIP] Update markdown-it version and relevant patches Update markdown-it version and relevant patches Jan 14, 2022
Copy link
Contributor

@ang-zeyu ang-zeyu left a comment

Choose a reason for hiding this comment

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

Lgtm! 🚀

@ang-zeyu ang-zeyu merged commit ead698c into MarkBind:master Jan 14, 2022
@ang-zeyu ang-zeyu added this to the 4.0 milestone Jan 14, 2022
@jonahtanjz jonahtanjz modified the milestones: 4.0, 3.1.1 Jan 22, 2022
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.

Unable to add multiple class names to hr element
3 participants