Skip to content

fix: CJK約物隣接の bold/em が壊れる問題を修正#62

Merged
myakura merged 2 commits intomainfrom
fix/cjk-punct-bold
Apr 28, 2026
Merged

fix: CJK約物隣接の bold/em が壊れる問題を修正#62
myakura merged 2 commits intomainfrom
fix/cjk-punct-bold

Conversation

@myakura
Copy link
Copy Markdown
Member

@myakura myakura commented Apr 28, 2026

Summary

  • **太字(strong)**にして のように CJK 閉じ括弧の直後に ** があり、その後に通常テキストが続くと bold が正しくレンダリングされない問題を修正
  • CommonMark 由来の制限(closing delimiter の right-flanking 判定)が原因で、marked はこれを仕様通りに実装しているため marked 側では修正されない
  • marked.use()emStrong をオーバーライドし、emStrongRDelimAst の lookahead に \p{L} を追加することで CJK 文字の前でも closing delimiter と認識されるよう対処

Test plan

  • npm test が全テスト通過することを確認
  • **foo(bar)**baz**qux**<strong>foo(bar)</strong>baz<strong>qux</strong> となることを確認

🤖 Generated with Claude Code

marked の emStrongRDelimAst regex は CommonMark 由来の制限により、
約物(`)` など)直後の `**` を closing delimiter として認識しない。
`marked.use()` で emStrong をオーバーライドし、group1 の lookahead に
`\p{L}` を追加することで、約物 + ** + 文字(CJK含む)の並びも
closing delimiter と見なすよう修正した。

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses an issue where closing bold delimiters are not correctly identified when following CJK punctuation by extending the marked tokenizer. Feedback points out a critical syntax error due to a missing Unicode flag and incorrect string escaping in the regex replacement. Additionally, it is recommended to use a scoped marked instance to avoid global side effects and to optimize the patching logic by moving it to the module level.

Comment thread lib/renderer/md.js
Comment on lines +14 to +17
this.rules.inline.emStrongRDelimAst = new RegExp(
orig.source.replace('(?=[\\s]|$)', '(?=[\\s\\p{L}]|$)'),
orig.flags
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

This regex modification has two significant issues:

  1. Missing u flag: The use of Unicode property escapes like \p{L} requires the u (unicode) flag to be set on the regular expression. Without this flag, new RegExp() will throw a SyntaxError in modern JavaScript environments (Node.js, modern browsers).
  2. Incorrect string escaping: In the replace call, the string literal '[\s]' results in [s] because the backslash is not escaped within the string. To correctly match the literal \s in the regex source, you must use '[\\s]' (double backslash).

Additionally, consider if \p{P} (punctuation) should also be included in the lookahead to fully support CommonMark's flanking rules for CJK punctuation (e.g., **foo(bar)**。).

Suggested change
this.rules.inline.emStrongRDelimAst = new RegExp(
orig.source.replace('(?=[\\s]|$)', '(?=[\\s\\p{L}]|$)'),
orig.flags
);
this.rules.inline.emStrongRDelimAst = new RegExp(
orig.source.replace('(?=[\\\\s]|$)', '(?=[\\\\s\\\\p{L}]|$)'),
orig.flags.includes('u') ? orig.flags : orig.flags + 'u'
);

Comment thread lib/renderer/md.js
Comment on lines +9 to +23
marked.use({
tokenizer: {
emStrong(src, maskedSrc, prevChar = '') {
if (!this._cjkFixed) {
const orig = this.rules.inline.emStrongRDelimAst;
this.rules.inline.emStrongRDelimAst = new RegExp(
orig.source.replace('(?=[\\s]|$)', '(?=[\\s\\p{L}]|$)'),
orig.flags
);
this._cjkFixed = true;
}
return false;
}
}
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Calling marked.use() at the top level of the module modifies the global marked instance. This side effect affects all other parts of the application that might use marked, which can lead to unexpected behavior or conflicts. It is recommended to use a scoped instance of marked (e.g., new marked.Marked()) and apply extensions to that instance instead of the global one.

Comment thread lib/renderer/md.js Outdated
Comment on lines +12 to +19
if (!this._cjkFixed) {
const orig = this.rules.inline.emStrongRDelimAst;
this.rules.inline.emStrongRDelimAst = new RegExp(
orig.source.replace('(?=[\\s]|$)', '(?=[\\s\\p{L}]|$)'),
orig.flags
);
this._cjkFixed = true;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The _cjkFixed flag is being set on this (the tokenizer instance). Since marked typically creates a new tokenizer instance for each parse, this patching logic will re-run every time a new parse begins and encounters an emStrong token. While the replace operation is idempotent here, it is more efficient to use a module-level variable or apply the patch once outside the tokenizer to avoid redundant regex compilation and property assignment on every parse.

`this` はパース毎に新しいインスタンスになるため、インスタンスプロパティとして
設定した `_cjkFixed` は毎回 `undefined` になり意図通りに機能しない。
モジュールスコープの変数に移動することで、最初のパースでのみパッチを適用する
という本来の意図を正しく実現する。

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@myakura
Copy link
Copy Markdown
Member Author

myakura commented Apr 28, 2026

@gemini-code-assist レビューありがとうございます。各指摘について検討した結果をお伝えします。

High priority(u フラグと文字列エスケープ)

こちらは対応しませんでした。

emStrongRDelimAst はすでに gu フラグを持っているため、\p{L} は問題なく動作します。また、現在のコードの '(?=[\\s]|$)' は JS ファイル上では (?=[\s]|$) という文字列になり、regex source 内のパターンと正しく一致します。

ご提案の '(?=[\\\\s]|$)'(4バックスラッシュ)は (?=[\\s]|$)(バックスラッシュ2つ)という文字列になり、regex source との一致に失敗してパッチが適用されなくなります。現行コードが正しいと判断しました。

Medium(グローバルな marked の副作用)

指摘の趣旨は理解しています。ただ、同ファイル内の code レンダラーのカスタマイズも同様に marked.use() でグローバルに適用するパターンを使っており、スコープを変えるにはより大きなリファクタリングが必要になるため、今回はスコープ外としました。

Medium(_cjkFixed の効率)

こちらは妥当な指摘でした。this はパース毎に新しいインスタンスになるため this._cjkFixed は意図通りに機能していませんでした。モジュールスコープの変数 let _cjkFixed に移動するよう修正しました。

@gemini-code-assist
Copy link
Copy Markdown

ご回答ありがとうございます。各点について承知いたしました。特に _cjkFixed のスコープ修正と、正規表現の挙動に関する詳細な説明をいただきありがとうございます。修正内容とテストケースを確認いたしました。

@myakura myakura merged commit dc27383 into main Apr 28, 2026
@myakura myakura deleted the fix/cjk-punct-bold branch April 28, 2026 09:58
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.

1 participant