mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-09-21 03:03:15 +08:00
fix(latex): do not re-escape a dollar that is already escaped (#15602)
## Description Split out of #15596 on review feedback — this is an independent, pre-existing bug in the currency pass, so it does not belong in that PR. The currency pass escapes any `$` followed by a digit without looking at what precedes it: ```ts preprocessedContent = preprocessedContent.replace(/\$(?=\d)/g, '\\$'); ``` When the dollar is **already escaped**, that turns an odd backslash run even: ``` input: \$4 (an escaped dollar — the author wants a literal "$4") output: \\$4 (a literal backslash, then an unescaped `$`) ``` `MarkdownPreprocessorManager.process` runs this before `_markdownToAst`, so remark-math sees that bare `$` and can take it as an active delimiter — the opposite of what escaping it was for. ### The fix Escape only a dollar whose preceding backslash run is **even**, keeping the run itself: ```ts preprocessedContent = preprocessedContent.replace( /(?<!\\)((?:\\\\)*)\$(?=\d)/g, '$1\\$' ); ``` `(?<!\\)` anchors the run to its start and `((?:\\\\)*)` takes backslashes in pairs, so an odd run never matches. Behaviour that was already correct is untouched: | input | before | after | |---|---|---| | `costs $5 today` | `costs \$5 today` | unchanged | | `costs \\$4 today` (even run — literal `\` then a real `$`) | `costs \\\$4 today` | unchanged | | `costs \$4 today` (odd run — already escaped) | `costs \\$4 today` | **`costs \$4 today`** | ### Relationship to #15596 Both touch this file, so whichever lands second will need a trivial rebase: - Both add `export` to `preprocessLatex` (the same one-word change). - They edit **different** statements — #15596 changes the *protection* regex above, this one changes the *currency* regex below. - Test files are deliberately named differently (`latex-preprocessor.unit.spec.ts` there, `latex-currency-escape.unit.spec.ts` here) so they do not collide. Happy to fold them into one file once both are in, if you'd prefer that. I verified this bug is not introduced by #15596: `\$4` preprocesses to `\\$4` identically with and without that branch. ## Checklist - [ ] I have signed the [AFFiNE Contributor License Agreement](https://cla-assistant.io/toeverything/AFFiNE) — required before merge; the `license/cla` check must be green ([how it works](https://github.com/toeverything/AFFiNE/blob/canary/docs/BUILDING.md#sign-the-cla-first)) - [x] The PR targets the `canary` branch and its title follows [Conventional Commits](https://www.conventionalcommits.org/) - [x] Tests are added or updated where it makes sense - [x] `yarn lint` and `yarn typecheck` pass locally - [x] If the PR code includes AI-generated edits, I have carefully reviewed it to ensure that the scope of the PR is consistent with what is claimed in the title and description, and that there are no redundant or invalid implementations. - [x] I agree that maintainers may close the PR without discussion if they consider the code quality to be too low. ### Notes on the checklist - **CLA:** not signed yet — the repository owner is signing it; the `license/cla` check will go green once that happens. - **Tests:** 7 cases in `latex-currency-escape.unit.spec.ts`. Reverting only the regex fails exactly the three already-escaped cases and leaves the four guards (plain price, bare amount, two prices, even run) green — they are regression guards, not props. Full `blocksuite/affine/all` suite: 230 passing. - **Lint/format:** `oxlint --deny-warnings` and `oxfmt --check` are both clean on the two changed files. - **Typecheck:** clean for these files. For transparency, a full `yarn typecheck` on my machine also reports pre-existing `@prisma/client` errors under `packages/backend/server`; those come from my installing with `--mode=skip-build` (so `prisma generate` never ran) and are unrelated to this change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved LaTeX handling for currency values by escaping dollar signs only when required. - Preserved already-escaped dollar signs and backslash sequences correctly. - **Tests** - Added coverage for plain currency amounts, already-escaped dollar signs, and even backslash runs. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1,30 @@
|
||||
import { preprocessLatex } from '@blocksuite/affine-block-latex';
|
||||
import { describe, expect, test } from 'vitest';
|
||||
|
||||
describe('latex preprocessor currency escaping', () => {
|
||||
test.each([
|
||||
['a plain price', 'costs $5 today', 'costs \\$5 today'],
|
||||
['a bare amount', '$4', '\\$4'],
|
||||
['two prices', 'costs $5 and $10', 'costs \\$5 and \\$10'],
|
||||
])('escapes %s', (_, markdown, expected) => {
|
||||
expect(preprocessLatex(markdown)).toBe(expected);
|
||||
});
|
||||
|
||||
describe('a dollar that is already escaped', () => {
|
||||
// Escaping it again turns the odd backslash run even, which leaves a
|
||||
// literal backslash followed by an unescaped `$` for remark-math.
|
||||
test.each([
|
||||
['one backslash', 'costs \\$4 today'],
|
||||
['three backslashes', '\\\\\\$4'],
|
||||
['several amounts', '\\$4 and \\$10'],
|
||||
])('is left alone with %s', (_, markdown) => {
|
||||
expect(preprocessLatex(markdown)).toBe(markdown);
|
||||
});
|
||||
});
|
||||
|
||||
test('an even backslash run still has its dollar escaped', () => {
|
||||
// `\\` is a literal backslash, so the `$` after it is unescaped and is a
|
||||
// genuine currency candidate. The run itself must survive untouched.
|
||||
expect(preprocessLatex('costs \\\\$4 today')).toBe('costs \\\\\\$4 today');
|
||||
});
|
||||
});
|
||||
@@ -32,7 +32,7 @@ function escapeMhchem(text: string) {
|
||||
* @param content - The content to preprocess
|
||||
* @returns The preprocessed content
|
||||
*/
|
||||
function preprocessLatex(content: string) {
|
||||
export function preprocessLatex(content: string) {
|
||||
// Protect code blocks
|
||||
const codeBlocks: string[] = [];
|
||||
let preprocessedContent = content;
|
||||
@@ -54,8 +54,17 @@ function preprocessLatex(content: string) {
|
||||
}
|
||||
);
|
||||
|
||||
// Escape dollar signs that are likely currency indicators
|
||||
preprocessedContent = preprocessedContent.replace(/\$(?=\d)/g, '\\$');
|
||||
// Escape dollar signs that are likely currency indicators.
|
||||
//
|
||||
// A dollar preceded by an odd number of backslashes is already escaped, so
|
||||
// adding another backslash makes the run even: `\$4` becomes `\\$4`, which is
|
||||
// a literal backslash followed by an unescaped `$` that remark-math can then
|
||||
// treat as a delimiter. Only escape a dollar whose preceding backslash run is
|
||||
// even, and keep that run as it was.
|
||||
preprocessedContent = preprocessedContent.replace(
|
||||
/(?<!\\)((?:\\\\)*)\$(?=\d)/g,
|
||||
'$1\\$'
|
||||
);
|
||||
|
||||
// Restore LaTeX expressions
|
||||
preprocessedContent = preprocessedContent.replace(
|
||||
|
||||
Reference in New Issue
Block a user