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

chore: textlintを導入 #83

Merged
merged 16 commits into from
Dec 11, 2024
Merged

chore: textlintを導入 #83

merged 16 commits into from
Dec 11, 2024

Conversation

3w36zj6
Copy link
Member

@3w36zj6 3w36zj6 commented Nov 27, 2024

変更点

  • miseの管理下にNode.jsとBunを追加する
  • BunでNode.jsのプロジェクトを初期化する
  • Node.jsの依存関係にtextlintを追加し、textlintの設定ファイルを整備する
  • 不要なルールを選定および無効化する
  • 自動修正を適用する
  • 自動修正できない表現を手動で修正する

確認事項

  • textlintが利用できる
    • bun run textlint-md./docs/のMarkdownファイルを静的解析できる
    • bun run textlint-md:fix./docs/のMarkdownファイルを自動修正できる
    • bun run textlint-html./dist/docs/のHTMLファイルを静的解析できる
  • CIでtextlintの静的解析が利用できる

備考

  • 本来は直接Rustのソースコードを静的解析するべきだが、プラグインが存在しないので出力されたHTMLを解析に利用している

@3w36zj6 3w36zj6 force-pushed the feature/add-textlint branch 4 times, most recently from c5e64cd to ec4e4e7 Compare November 27, 2024 15:41
@3w36zj6 3w36zj6 force-pushed the feature/add-textlint branch 2 times, most recently from 1f05a1f to 2067c17 Compare November 27, 2024 17:29
@kimushun1101
Copy link
Contributor

不要なルールの選定および無効化、自動修正の適用は行っていない

CIが失敗しているのはこちらの影響でしょうか?ルールを設定ファイルなどで書く方法は無いんですかね?

@3w36zj6
Copy link
Member Author

3w36zj6 commented Nov 29, 2024

不要なルールは.textlintrcで個別に無効化できるため、プロジェクト内で一貫して無視すると決まったルールはこちらの設定ファイルに記述すると良いと思います。
また、文書内で部分的にルールを無視したい場合はtextlint-filter-rule-commentsで明示的に管理できます。

@3w36zj6 3w36zj6 force-pushed the feature/add-textlint branch 2 times, most recently from cd32d52 to fd550a9 Compare November 30, 2024 20:19
@3w36zj6
Copy link
Member Author

3w36zj6 commented Nov 30, 2024

ルールの選定や自動修正できない表現の修正は多くの議論が必要になると思われるので、CIはcontinue-on-error: trueを追加してtextlintの終了コードに関わらず正常終了するようにし、当分はtextlintの解析結果は参考程度とする運用が良いかと思いましたが、いかがでしょうか?

@kimushun1101
Copy link
Contributor

ルールの選定や自動修正できない表現の修正は多くの議論が必要になると思われるので、CIはcontinue-on-error: trueを追加してtextlintの終了コードに関わらず正常終了するようにし、当分はtextlintの解析結果は参考程度とする運用が良いかと思いましたが、いかがでしょうか?

ひとまずこちらの対応で問題無いと思います。

Node.jsに不慣れなためお伺いしたいのですが、bun run textlint-mdコマンドなどを実行するための下準備などはありますでしょうか?
mise installmise run generateをしたあとに、同じターミナルでbun run textlint-mdコマンドを入力したのですが、以下のようなエラーが出てしまいました。

$ textlint ./docs/
/usr/bin/bash: line 1: textlint: command not found
error: script "textlint-md" exited with code 127

基本的な内容かもしれませんが、教えていただけますと幸いです。

@3w36zj6
Copy link
Member Author

3w36zj6 commented Dec 1, 2024

説明が不十分で失礼しました。Node.jsの依存関係が更新される度に以下のコマンドを実行が必要です。

bun install --frozen-lockfile

Node.jsのプロジェクトでは、プロジェクトの依存関係はnode_modules/というディレクトリで管理されます。CLIツールの実行やスクリプト内でパッケージをimportする際にはこのディレクトリが参照されますが、初期の状態ではこのディレクトリが存在していないためエラーが発生しています。

また、Node.jsには標準のパッケージマネージャーであるnpmが同梱されていますが、パフォーマンスの観点などを理由にBunというサードパーティのパッケージマネージャーを選定しています。

.textlintrc.js Outdated Show resolved Hide resolved
@3w36zj6
Copy link
Member Author

3w36zj6 commented Dec 2, 2024

jtf-style/3.3.かっこ類と隣接する文字の間のスペースの有無のルールの自動修正の問題ですが、英文に対して適用することが問題なのではなく、Typstに同梱されているMarkdownファイルのマークアップが不完全であり、remarkがリンク定義の存在しない[]囲いをリンクとして解釈できないことが問題のようです。

@3w36zj6
Copy link
Member Author

3w36zj6 commented Dec 2, 2024

既存のMarkdownファイルのtextlintで誤検出が起こるハイパーリンクを修正し、textlintの自動修正を適用しました。

@3w36zj6 3w36zj6 requested a review from kimushun1101 December 2, 2024 07:33
Copy link
Contributor

@kimushun1101 kimushun1101 left a comment

Choose a reason for hiding this comment

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

当分はtextlintの解析結果は参考程度とする運用が良いかと思いましたが、いかがでしょうか?

参考程度ということですので厳密に行う必要はないとは思いますが、bun run textlint-mdであと3つエラーが残っています。bun run textlint-md:fixでは自動で修正されないため手作業で修正する必要がありそうです。

$ textlint ./docs/

/home/kimura/typst_ws/typst-jp.github.io/docs/reference/scripting.md
  214:1  error  箇条書きの文末から句点(。)を外して下さい。
箇条書きの文末に句点(。)を付けるかを統一します。  jtf-style/1.1.3.箇条書き
  263:1  error  箇条書きの文末から句点(。)を外して下さい。
箇条書きの文末に句点(。)を付けるかを統一します。  jtf-style/1.1.3.箇条書き

/home/kimura/typst_ws/typst-jp.github.io/docs/reference/context.md
  18:25  error  本文を敬体(ですます調)に統一して下さい。
本文の文体は、敬体(ですます調)あるいは常体(である調)のどちらかで統一します。
"であるから"が常体(である調)です。  jtf-style/1.1.1.本文

✖ 3 problems (3 errors, 0 warnings)

error: script "textlint-md" exited with code 1

1つ目は箇条書きであるため句点を外せとのことです。ただし文章として成立しているので、どう外したらよいか悩みどころです。

対象の値は以下のいずれかです。
- 指定されたキーを持つ[辞書]($dictionary)
- 指定された修飾子を持つ[記号]($symbol)
- 指定された定義を含む[モジュール]($module)
- 指定されたフィールドを持つ要素で構成された[コンテンツ]($content)。利用可能なフィールドは、その要素が構築された際に与えられた[要素関数]($function/#element-functions)の引数と一致します。

2つ目は上記と同様にインクルードの説明のところの句点を外せとのことです。言われた通りに外せばエラーは消えますが、文章となっているため句点を外すかは悩みどころです。また直後のインポートの説明は2文になっているため免れている感じですかね?あまり判断基準がわかっていないため、もしパッとわかるようであれば教えてください。

- **インクルード:** `{include "bar.typ"}` \
パス`bar.typ`にあるファイルを評価し、その結果として得られる[コンテンツ]($content)を返します。
- **インポート:** `{import "bar.typ"}` \
パス`bar.typ`にあるファイルを評価し、その結果として得られる
[モジュール]($module)を現在のスコープに`bar`(拡張子なしのファイル名)として挿入します。次のように、`as`キーワードを使用してモジュール名を変更できます。

3つ目は簡単に対応できそうであったため、修正提案を行いました。


もしご存知でしたら教えていただきたいのですが、bun run textlint-md:fixの処理時間が若干長く感じます。おそらく上記のような、自動修正できないようなものを一生懸命考えているのでしょうか?それともすべてのファイルを見ているから遅いのであり、例えば、対象ファイルを指定して実行するなどは可能でしょうか?

docs/reference/context.md Outdated Show resolved Hide resolved
@kimushun1101
Copy link
Contributor

レビューをCommentで提出いたしましたが、

当分はtextlintの解析結果は参考程度とする運用が良いかと思いましたが、いかがでしょうか?

これに同意しているため、ひとまず本PRの目的は達成したとして、いまいま判断が難しいところは別PRに回して頂いてもよいかとも感じました。ご感想を伺えますと幸いです。

3w36zj6 and others added 2 commits December 8, 2024 15:35
Co-authored-by: Shunsuke KIMURA <kimushun1101@gmail.com>
@3w36zj6
Copy link
Member Author

3w36zj6 commented Dec 8, 2024

原文を忠実に訳した結果、ルールを破る必要が生じる部分にtextlint-disableのコメントを追加しました。

@3w36zj6 3w36zj6 requested a review from kimushun1101 December 8, 2024 09:24
@3w36zj6
Copy link
Member Author

3w36zj6 commented Dec 8, 2024

Node.jsのプロジェクトに関するタスクの定義はpackage.jsonscriptsで管理されており、実際に実行しているコマンドは以下の通りです。

{
  "scripts": {
    "textlint-md": "textlint ./docs/",
    "textlint-md:fix": "textlint --fix ./docs/",
    "textlint-html": "textlint ./dist/docs/"
  }
}

textlintの解析の遅さが煩わしい場合は、変更を加えたファイルまたはフォルダのみを解析の対象にすると良いと思います。ここで、node_modules/で管理されているpackageを実行するにはbun <command>とする必要があるため、bun textlint ./path/to/などで特定のファイルに対してtextlintを実行できます。

Husky+lint-stagedLefthookのような、コミット前に変更を加えたファイルのみにフォーマットやリントを自動で適用するツールもあるので、需要があれば整備しても良いかもしれません。

Signed-off-by: Shunsuke Kimura <kimushun1101@gmail.com>
Copy link
Contributor

@kimushun1101 kimushun1101 left a comment

Choose a reason for hiding this comment

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

ありがとうございました。よく理解できました。

Husky+lint-stagedLefthookのような、コミット前に変更を加えたファイルのみにフォーマットやリントを自動で適用するツールもあるので、需要があれば整備しても良いかもしれません。

そうですね。翻訳の性質上、1度のプルリクエストであまり多くのファイルを変更されないと思いますので、ひとまずは様子見でよいと思います。

bun run textlint-htmlも試してみました。
対応として 440408e で修正を加えました。

$ textlint ./dist/docs/

/home/kimura/work/typst-jp.github.io/dist/docs/reference/model/cite/index.html
  1472:50  ✓ error  かっこの外側、内側ともにスペースを入れません。                          jtf-style/3.3.かっこ類と隣接する文字の間のスペースの有無
  1481:50  ✓ error  かっこの外側、内側ともにスペースを入れません。                          jtf-style/3.3.かっこ類と隣接する文字の間のスペースの有無
  1490:50  ✓ error  かっこの外側、内側ともにスペースを入れません。                          jtf-style/3.3.かっこ類と隣接する文字の間のスペースの有無
  1499:50  ✓ error  かっこの外側、内側ともにスペースを入れません。                          jtf-style/3.3.かっこ類と隣接する文字の間のスペースの有無
  1661:23  ✓ error  半角のかっこ()が使用されています。全角のかっこ()を使用してください。  jtf-style/4.3.1.丸かっこ()

/home/kimura/work/typst-jp.github.io/dist/docs/reference/model/bibliography/index.html
  1408:63  ✓ error  半角のかっこ()が使用されています。全角のかっこ()を使用してください。  jtf-style/4.3.1.丸かっこ()
  1417:67  ✓ error  半角のかっこ()が使用されています。全角のかっこ()を使用してください。  jtf-style/4.3.1.丸かっこ()
  1426:60  ✓ error  半角のかっこ()が使用されています。全角のかっこ()を使用してください。  jtf-style/4.3.1.丸かっこ()
  1435:63  ✓ error  半角のかっこ()が使用されています。全角のかっこ()を使用してください。  jtf-style/4.3.1.丸かっこ()
  1597:23  ✓ error  半角のかっこ()が使用されています。全角のかっこ()を使用してください。  jtf-style/4.3.1.丸かっこ()

✖ 10 problems (10 errors, 0 warnings)
✓ 10 fixable problems.
Try to run: $ textlint --fix [file]

まだ、citeとbibliographyのページでエラーが残りますが、これらはまだ翻訳前(原文)の部分です。

該当箇所はいずれもstyleの章のView optionsで現れる参考文献の体裁のリストで、おそらくここから引っ張って来られていそうなので、手の出しようが無い気がします。
https://github.com/typst/hayagriva/blob/38a56c8dea69531befea140aa51ca913fe9ca586/src/csl/archive.rs#L172-L184

@3w36zj6
Copy link
Member Author

3w36zj6 commented Dec 10, 2024

jtf-style/4.3.1.丸かっこ()は日本語のみを対象にしていますが1中文が日本語と判定されているようです。.textlintignoreに該当のファイルを追加し、エラーを抑制しました。

Footnotes

  1. https://github.com/textlint-ja/textlint-rule-preset-JTF-style/blob/master/src/util/regexp.js#L3

@kimushun1101 kimushun1101 mentioned this pull request Dec 10, 2024
3 tasks
@kimushun1101
Copy link
Contributor

素晴らしいです!
最新版でmise run generateをしたあと、bun run textlint-mdbun run textlint-htmlのいずれもエラーが無い状態となりました。改めてapprovedです。

注意点として

この2つだけレビューを気をつける必要がありそうですが、時間があるときに私が対応してしまいますね。

Copy link
Contributor

@mkpoli mkpoli left a comment

Choose a reason for hiding this comment

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

LGTM

検査や修正が正しく作動することを確認できました

@3w36zj6 3w36zj6 merged commit 41be5a3 into main Dec 11, 2024
3 checks passed
@3w36zj6 3w36zj6 deleted the feature/add-textlint branch December 11, 2024 12:57
This was referenced Dec 14, 2024
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.

3 participants