-
Notifications
You must be signed in to change notification settings - Fork 138
Make beg_end math delimitor default #2737
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR makes the beg_end math delimiter available by default without requiring explicit configuration in site.json. The change simplifies the user experience by automatically enabling support for LaTeX \begin{equation}...\end{equation} syntax.
- Introduces a new default plugin that automatically applies the
beg_endmath delimiter - Removes the need for users to manually configure
mathDelimitersplugin for basic equation support - Maintains backward compatibility with existing delimiter configurations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/core/src/plugins/default/markbind-plugin-begendMathDelimitor.ts |
New plugin that automatically applies beg_end delimiter using markdown-it-texmath |
packages/core/test/unit/plugins/default/begendMathDelimitor.test.ts |
Test coverage for the new default plugin functionality |
Comments suppressed due to low confidence (1)
packages/core/test/unit/plugins/default/begendMathDelimitor.test.ts:7
- The
jest.resetModules()call may not effectively reset thepluginAddedflag since it's a module-level variable. Consider testing the plugin's idempotent behavior more explicitly.
jest.resetModules();
| import katex from 'katex'; | ||
| import md from '../../lib/markdown-it'; | ||
|
|
||
| const texmath = require('markdown-it-texmath'); |
Copilot
AI
Aug 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ES6 import syntax instead of require() to maintain consistency with the other imports in the file.
| const texmath = require('markdown-it-texmath'); | |
| import * as texmath from 'markdown-it-texmath'; |
| beforeSiteGenerate: () => { | ||
| applyBegEndDelimiters(); | ||
| }, | ||
| }; |
Copilot
AI
Aug 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global pluginAdded flag could cause issues in multi-threaded environments or when the module is imported multiple times. Consider using a more robust state management approach.
| }; | |
| const plugin = { | |
| pluginAdded: false, | |
| applyBegEndDelimiters() { | |
| if (!this.pluginAdded) { | |
| // Always use 'beg_end' delimiter regardless of site.json config | |
| md.use(texmath, { | |
| engine: katex, | |
| delimiters: ['beg_end'], // force beg_end delimiter | |
| }); | |
| this.pluginAdded = true; | |
| } | |
| }, | |
| beforeSiteGenerate() { | |
| this.applyBegEndDelimiters(); | |
| }, | |
| }; | |
| export = plugin; |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2737 +/- ##
==========================================
+ Coverage 52.84% 52.92% +0.07%
==========================================
Files 130 131 +1
Lines 7162 7173 +11
Branches 1591 1502 -89
==========================================
+ Hits 3785 3796 +11
Misses 3072 3072
Partials 305 305 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Golddirio for your PR!
The goal of enabling beg_end delimiters by default makes sense, but the approach here feels heavier than necessary.
MarkBind’s markdown pipeline already initializes markdown-it with its math delimiters in a single, central location. Instead of introducing a new plugin (with its own state, lifecycle hook, and tests), it might be worth looking at where the default delimiter list is set during markdown-it setup. That’s the same place our other delimiters (dollar, brackets) are configured.
By adjusting that initialization directly, we could achieve the same effect without adding an extra plugin layer or lifecycle dependency. This would also keep delimiter logic in one place, consistent with the architecture described here.
See: MarkBind Architecture
See: supported delimiters
Edit: Additionally, consider updating the docs as well to reflect any new changes!
What is the purpose of this pull request?
Closes #2710
Overview of changes:
The
beg_endMath delimitor is made default without the need to configure insite.jsonfor users. Hence, the following code is no longer needed if the user would like to usebeg_end:{ "plugins": [ "mathDelimiters" ], "pluginsContext" : { "mathDelimiters" : { "delimiters": ["beg_end"] } }, }Anything you'd like to highlight/discuss:
This improvement does not affect the use of other Math delimitors. If the user would like to use other delimitors other than
beg_endthey still need to configure insite.jsonas usual.Testing instructions:
Proposed commit message: (wrap lines at 72 characters)
Make beg_end math delimitor default
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major,r.Minor,r.Patch.Breaking change release note preparation (if applicable):