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

Truncate newline between non-ASCII characters #61

Merged
merged 3 commits into from
Oct 1, 2019

Conversation

r7kamura
Copy link
Contributor

@r7kamura r7kamura commented Nov 17, 2018

解決したい問題

rurema/doctree#1579 で話していた問題への対応です。

ソースコードの編集上の都合で、入力される文章の中に改行が含まれている場合がありますが、Web ブラウザの CSS Text Module Level 3 への対応状況がまちまちなため、幾つかの Web ブラウザでは日本語文章の中でこの改行が空白として利用者に見える形で表示されてしまうという問題があります。

解決案

概要

ソースコードの編集上の都合で含まれている (HTML として出力する上では) 不要な改行を、「マルチバイト文字の間の改行は不要である」ということを根拠に bitclust 側で取り除くことで、この問題に対処します。

実装案1

BitClust::Preprocessor#wrap では、入力文字列を行単位で処理しています。このとき、

  1. 現在処理中の行がマルチバイト文字列に続く改行で終わっている
  2. 次に処理する行がマルチバイト文字で始まっている

という条件が同時に満たされる場合、現在処理中の行から末尾の改行を取り除きます。これにより、マルチバイト文字の間の改行が取り除かれるのではという寸法です。

次の行を見て判断する必要があるため、判断が必要になり次第先に次の行を読んでしまうというようなロジックを加えています。(つまり peek している)

実装案2

この Pull Request のコメント欄にあるように、実装案1では幾つかの解決できない問題があるため、BitClust::Preprocessor ではなく BitClust::RDCompiler の挙動を変更したのが実装案2です。

まず最初に、複数行からなる入力文字列を HTML 向けのテキストノード (実際はタグの含まれないただの文字列) に変換している箇所を、必ず #text_node_from_lines で処理するようにリファクタリングしました (但し pre 要素向けのテキストノードを生成する際にはこれを利用していません)。この処理は、以下の箇所で発生するようです (それぞれの構文規則の正確な名前は分からなかったので適当な名前を充てています)。

  • definition list
  • paragraph
  • entry info
  • entry paragraph
  • entry see

その上で、マルチバイト文字の間の改行を取り除く処理を #text_node_from_lines に加えました。

以下のテストケースを追加しています。それぞれの構文規則ごとに、英語と日本語を渡したときの挙動をテストしています。paragraph についてだけは、更に詳細な条件でテストしています (paragraph で成功すれば同じ実装を利用している他の箇所でも成功する可能性が高いだろうという考えから)。

  • TestRDCompiler#test_definition_list_with_ascii
  • TestRDCompiler#test_definition_list_with_non_ascii
  • TestRDCompiler#test_entry_info_with_ascii
  • TestRDCompiler#test_entry_info_with_non_ascii
  • TestRDCompiler#test_entry_paragraph_with_ascii
  • TestRDCompiler#test_entry_paragraph_with_non_ascii
  • TestRDCompiler#test_entry_see_with_ascii
  • TestRDCompiler#test_entry_see_with_non_ascii
  • TestRDCompiler#test_paragraph_with_newline_between_ascii_and_ascii
  • TestRDCompiler#test_paragraph_with_newline_between_ascii_and_non_ascii
  • TestRDCompiler#test_paragraph_with_newline_between_non_ascii_and_ascii
  • TestRDCompiler#test_paragraph_with_newline_between_non_ascii_and_non_ascii
  • TestRDCompiler#test_paragraph_with_single_line

This is a temporary measure for incomplete Web browsers that cannot
treat newline character between CJK multi-byte characters for now.

This change is related to rurema/doctree#1579.
@scivola
Copy link
Contributor

scivola commented Nov 17, 2018

おお!

ところで,これだとたぶん,
https://github.com/rurema/doctree/blob/master/refm/api/src/_builtin/Float#L120-L121
みたいな箇所に対応できませんよね。

@r7kamura
Copy link
Contributor Author

r7kamura commented Nov 18, 2018

これだとたぶん,
https://github.com/rurema/doctree/blob/master/refm/api/src/_builtin/Float#L120-L121
みたいな箇所に対応できませんよね。

うわっ、その通りですね。ありがたい指摘。

rurema/doctree#1579 のやり取りだと確か Preprocessor を修正する話が出ていたので、あまり上手く考えがまとまらないまま変更してみた感じでした。しかし Web ブラウザ向けの対応なので、単純に考えると HTML への整形時に改行を取り除くというのが妥当そうで、BitClust::RDCompiler が doc を HTML にコンパイルする担当のクラスなので、BitClust::Preprocessor よりこちらを修正するのがやはり適切なんじゃないかと考えています。(問題は不具合が起きると大変ということですね...)

現状のコードでは BitClust::Proprocessor で pragma 以外の行を対象に処理を加えていましたが、BitClust::RDCompiler の処理を変更する形で実装し直すなら、 pre 要素などの特例以外のテキストノードを出力する箇所でマルチバイト文字列に関する処理を加えれば良いのかな。

@r7kamura
Copy link
Contributor Author

BitClust::Preprocessor ではなく BitClust::RDCompile の挙動を変更するように書き換えてみました。Pull Request の description も更新して、最初に書いたコードのほうを実装案1、次に書き換えたコードのほうを実装案2としました。

Copy link
Member

@sho-h sho-h left a comment

Choose a reason for hiding this comment

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

非常に遅くなりましたけど僕の方からは特に気になる点はありませんでしたー。

@sho-h sho-h added the 1.2.3 1.2.3でリリース予定 label Aug 24, 2019
@znz znz merged commit a98ec63 into rurema:master Oct 1, 2019
@r7kamura r7kamura deleted the feature/spacing branch October 1, 2019 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.2.3 1.2.3でリリース予定
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants