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

表示崩れを事前修正するMarkdown拡張を追加 #8

Merged
merged 9 commits into from
Dec 16, 2024

Conversation

faithandbrave
Copy link
Member

影響範囲が大きいので、一旦Pull Requestにします。
どなたかレビューしてもらえると助かりますが、とくにご意見なければ一週間ほどでマージします。

site_generator側は、この拡張を使うよう1行追加するだけです。

diff --git forkSrcPrefix/run.py forkDstPrefix/run.py
index 3a783c5504a7856e048800b357fa4698f0f7e9b4..25ac1f1118bf307dcd187a5435e66e76557f793f 100755
--- forkSrcPrefix/run.py
+++ forkDstPrefix/run.py
@@ -102,6 +102,7 @@ def md_to_html(md_data, path, hrefs=None, global_qualify_list=None, global_defin
         'markdown_to_html.mark',
         'markdown_to_html.sponsor',
         'markdown_to_html.commit',
+        'markdown_to_html.fix_display_error',
         ],
         extension_configs=extension_configs)
     md._html_attribute_hrefs = hrefs

@faithandbrave
Copy link
Member Author

追加で、コードブロック後に箇条書き以外がきたら空行を挟もうとしてますが、Markdown拡張の適用順の問題かうまく効いてくれない状態です

@faithandbrave
Copy link
Member Author

Registryにpriorityを設定できるみたいです。

いま3.2.1で使ってるadd関数はなくなるみたいなので、register関数に移行する必要がありそうです。

https://github.com/Python-Markdown/markdown/blob/3.2.1/markdown/util.py
https://github.com/Python-Markdown/markdown/blob/3.7/markdown/util.py

@faithandbrave
Copy link
Member Author

コードブロック後に箇条書き以外がきたら空行を挟むよう修正し、Registry.addを使っていたコードをRegistry.registerに直しました

@akinomyoga
Copy link
Member

akinomyoga commented Dec 9, 2024

コードブロック直後の文章消失の問題は

QUALIFIED_FENCED_BLOCK_RE = re.compile(r'(?P<fence>`{3,})[ ]*(?P<lang>[a-zA-Z0-9_+-]*)(?P<lang_meta>.*?)\n(?P<code>.*?)(?<=\n)(?P<indent>[ \t]*)(?P=fence)[ ]*\n(?:(?=\n)|(?P<qualifies>.*?\n(?=\s*\n)))', re.MULTILINE | re.DOTALL)

の方を修正して根治できませんか。(?P<qualifies>.*?\n(?=\s*\n)).*? の部分が何にでも一致してしまうのがよくないのだと思います。それとは別の preprocessor で何か警告など発する仕組みはあって良いと思います。

追記: 単に (?=\n)(?!\* ) [や (?!(?:[*+-]|[0-9]+\.)\s ) など?] にすれば良いだけかも。

@faithandbrave
Copy link
Member Author

@akinomyoga さんの方で修正できますか?

そうしたらこのPRでは箇条書きの前に空行を入れる修正だけ入れます

@akinomyoga
Copy link
Member

OK 後でやってみますね

akinomyoga added a commit to akinomyoga/cpprefjp-markdown_to_html that referenced this pull request Dec 10, 2024
Ref. [1] で発見された問題。コードブロック後に改行を挟まずに文章を続け
るとその文章がコード修飾指定のセクションとして抽出されて消失する問題。
対処法として改行を挿入する [2, 3] ことで回避できるが、改行がなくても正
しく表示されるようにしたい[4]。ここでは、コード修飾の指定についてより
厳密な形式で抽出を行い、誤って関係ない物を抽出することを防ぐ。

この変更のテストの過程で、既存のコード修飾の誤りが発見されたが対処した
[5]。また、他にも消失している記述 [6] が発見されたが、[6] はこの変更に
より自動的に修正されるので対処はしていない。

References:

[1] cpprefjp/site#1362 (comment)
[2] cpprefjp/site@c747f4a
[3] cpprefjp/site@5259ff6
[4] cpprefjp#8 (comment)
[5] cpprefjp/site@ebf8c8f
[6] https://github.com/cpprefjp/site/blob/ebf8c8fd705a194adb7b3f83786dad9c843d143b/reference/generator/generator/iterator/op_increment.md?plain=1#L27
akinomyoga added a commit to akinomyoga/cpprefjp-markdown_to_html that referenced this pull request Dec 10, 2024
Ref. [1] で発見された問題。コードブロック後に改行を挟まずに文章を続け
るとその文章がコード修飾指定のセクションとして抽出されて消失する問題。
対処法として改行を挿入する [2, 3] ことで回避できるが、改行がなくても正
しく表示されるようにしたい。ここでは、コード修飾の指定についてより厳密
な形式で抽出を行い、誤って関係ない物を抽出することを防ぐ [4]。

この変更のテストの過程で、既存のコード修飾の誤りが発見されたが対処した
[5]。また、他にも消失している記述 [6] が発見されたが、[6] はこの変更に
より自動的に修正されるので対処はしていない。

References:

[1] cpprefjp/site#1362 (comment)
[2] cpprefjp/site@c747f4a
[3] cpprefjp/site@5259ff6
[4] cpprefjp#8 (comment)
[5] cpprefjp/site@ebf8c8f
[6] https://github.com/cpprefjp/site/blob/ebf8c8fd705a194adb7b3f83786dad9c843d143b/reference/generator/generator/iterator/op_increment.md?plain=1#L27
@faithandbrave
Copy link
Member Author

コード修飾を処理したあと (コードブロック後の*箇条書きが消えたあと) に、箇条書き前に空行を入れるようにしました。
これでよさそうなら、Squash Mergeしようかと思います

Comment on lines 49 to 69
is_outer_code_block = line.strip().startswith("````")
if is_outer_code_block:
in_outer_code_block = not in_outer_code_block
prev_line = line
new_lines.append(line)
continue

if not in_outer_code_block:
is_code_block = line.strip().startswith("```")
if is_code_block:
in_code_block = not in_code_block
if not in_code_block:
is_prev_code_block = True
prev_line = line
new_lines.append(line)
continue

if in_code_block:
prev_line = line
new_lines.append(line)
continue
Copy link
Member

Choose a reason for hiding this comment

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

コードブロックは qualified_fenced_code 29 によって変換された後で既に存在しなくなっているのでこの部分は処理しなくて良いかなと思います。実際に手元でこの部分を全部コメントアウトして全変換して結果を比べてみましたが、変換結果に違いはありませんでした。

Copy link
Member Author

Choose a reason for hiding this comment

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

そうでした。直します

Copy link
Member Author

Choose a reason for hiding this comment

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

直しました。手元で確認しています

Copy link
Member

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

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

後、手元で変換して気づいたのは、

- リスト項目1 複数行1行目
  リスト項目1 複数行2行目
- リスト項目2

の形になっていた時に

- リスト項目1 複数行1行目
  リスト項目1 複数行2行目

- リスト項目2

に変換されて、リスト項目1の内部に新しく段落が生成されるようになっています。直近のリスト項目のインデントレベルを記録して、is_item_line の判定でインデントされた行も項目の一部という判定を加えて良いのではないかと思います。

@faithandbrave
Copy link
Member Author

なるほど、たしかにそれは考慮してなかったです

@faithandbrave
Copy link
Member Author

先頭4スペース以上の行とタブ開始の行も、箇条書きの行とみなすよう修正しました

@akinomyoga
Copy link
Member

akinomyoga commented Dec 11, 2024

変換結果を見てみたのですが先頭に空白2文字しかない場合もあります…というより先頭に何もない以下のような場合も結構あります。

- リスト項目1
リスト項目1の続き
- リスト項目2

先のコメントでインデントで判定するという様に提案しましたが、実際の Markdown の挙動を勘違いしていまして、すみません。"空白4文字" というのはリストの中に次の階層のリストを入れる場合の話であって、それが一般にリスト項目の中のインデントを規定する訳ではないみたいです。

これに関連して単に無駄に段落が生成されるだけでなくて構造が変わってしまっている箇所もあるみたいです。

これについてはどのように対処するのが良いでしょうか。Markdown ソースの方を空白4文字でインデントする様に修正するか、fix_display_error の方で Markdown の規則に合うように処理するか。

ソースの方を修正したら良いとも思ったのですが、既存の Markdown 実装にない cpprefjp 独自ルール 1 を新しく増やす事になるのではということ、実際にリスト項目の継続行をインデントしなくても動く場合と動かない場合があって振る舞いが一定せず表面的に意味不明な振る舞いに見えること、などを考えるとやはり微妙かなという気がしてきました。

追記: でも fix_diplay_error の方で適切に空行を入れるにしても、Python-Markdown のリストの正確なルールが分からないと結局また穴が残るかもしれず、それはそれで微妙な気もしてきました。

Footnotes

  1. ただし Markdown ではなくて wiki 文法ではインデントが必要

@faithandbrave
Copy link
Member Author

明日以降、現状調査からやります!

@faithandbrave
Copy link
Member Author

- リスト項目1
リスト項目1の続き
- リスト項目2

このケースを考慮するのであれば、箇条書きがはじまったら空行がくるまで箇条書きが続く、とみなすしかないような気がしますね

@akinomyoga
Copy link
Member

akinomyoga commented Dec 12, 2024

このケースを考慮するのであれば、箇条書きがはじまったら空行がくるまで箇条書きが続く、とみなすしかないような気がしますね

上記の方法で良さそうです。リスト項目の中に空行が含まれる場合はどうなるのだろうと思って Python-Markdown の振る舞いを色々調べてみたのですが、何か不可解な挙動をしていて、却って "箇条書きがはじまったら空行がくるまで箇条書きが続く" という判定による修正が一番正しそうです。

先ず、リスト項目内部に空行がない場合は、次のリスト項目は、インデントやリスト階層がどうあってもちゃんとリスト項目になるみたいです。

- リスト項目一行目
    二行目
    三行目
- これはリスト項目になる

- リスト項目一行目
    二行目
    三行目
    - これもリスト項目になる

- リスト項目一行目
二行目
三行目
- これもリスト項目になる

- リスト項目一行目
二行目
三行目
    - これもリスト項目になる

ところが、リスト項目の内部に空行がある場合には、次に来るリスト項目はどうあっても空行直後でしかリスト項目にならないみたいです。


- リスト項目一行目
    二行目

    三行目
- これはリスト項目に**ならない**

- リスト項目一行目
    二行目

    三行目
    - これもリスト項目に**ならない**

- リスト項目一行目
二行目

三行目
- これもリスト項目に**ならない**

- リスト項目一行目
二行目

三行目
    - これもリスト項目に**ならない**
  • 空白文字だけしかない行も空行と判定される
  • 空行の他に見出し行 /^#+ / も箇条書き終了になる

@faithandbrave
Copy link
Member Author

そのように直しました。
これで一通り確認してみます。

@akinomyoga
Copy link
Member

akinomyoga commented Dec 12, 2024

こちらでも見てみましたところちゃんと綺麗に変換できています!


reference/thread/jthread/op_constructor.md が壊れているみたいですが、これはそもそも Markdown の時点で壊れている気がします。以下の部分で 68行目, 69行目 の空行は間違って入っている気がします。67行目の文が尻切れトンボになっていますが、本当は70行目が文の続きですよね。

https://github.com/cpprefjp/site/blob/587d612b99f621e8cb0777e689ee0247d055d750/reference/thread/jthread/op_constructor.md?plain=1#L67-L71


あと、これは以前から壊れていてこの fix_display_error でも変わっていないものですが、以下の (3): のリストレベルが意図とは違ったもので生成されていますね。

https://github.com/cpprefjp/site/blob/587d612b99f621e8cb0777e689ee0247d055d750/reference/thread/jthread/op_constructor.md?plain=1#L43-L63

少なくとも Python-Markdown 3.2.1 は以下の記述に対して

- リスト項目1 段落1

    段落2

    - リスト項目1-1
- リスト項目2
  • リスト項目1 段落1

    段落2

    • リスト項目1-1
    • リスト項目2

という具合のリストを生成するみたいです。Python-Markdown で正しく表示させるためにはリスト項目2 の前にも空行を入れて、

- リスト項目1 段落1

    段落2

    - リスト項目1-1

- リスト項目2

としなければならないみたいです。

空行を跨いだリスト項目の判定も加えたらこれに対処することも可能かもしれませんが、正直これは Python-Markdown の欠陥のように思われるし、そこまで fix_display_error 面倒を見る必要もないと思うので、こういうのは手動で直すというのでOKと思います。

他に同様の間違いが発生している場所がないか検出しようとコードを少し弄ったら、ちゃんと動きそうなコードができました… akinomyoga@96132c3 ちなみに上記 reference/thread/jthread/op_constructor.md 以外には問題になっている箇所は見つかりませんでした。

@faithandbrave
Copy link
Member Author

確認ありがとうございます!
追加で問題が見つからなければ、週明けくらいにマージしようかと思います!

@faithandbrave faithandbrave merged commit e2af656 into master Dec 16, 2024
@faithandbrave faithandbrave deleted the fix_display_error branch December 16, 2024 01:42
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.

2 participants