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

Breaking Change: 文体が統一されていてもpreferIn設定に違反する場合エラーとなるように変更 #40

Merged
merged 4 commits into from
Dec 28, 2023

Conversation

yo1000
Copy link
Contributor

@yo1000 yo1000 commented Dec 27, 2023

現行では、"ですます" に統一しなければならない、といった文書の表現規則を事前に設けることができません。
あくまで記述された文章の結果表現が、"ですます"、ないし "である" に統一されていることだけが検査されます。

執筆前に、これから記述する文書は "ですます" に統一しなければならない、といった規則を設けたい場合があります。

こういった事前定義規則への違反を検知できるようにするため、デフォルトの動作を変更しました。
変更後は、preferIn の設定に従わない場合についても、等しくエラーとなります。

こういった事前定義規則への違反を検知できるようにするためオプションを追加しました。
オプションを使用すると、preferIn に設定された表現と一致しない場合に、エラーとなります。

{
    "rules": {
        "no-mix-dearu-desumasu": {
             "preferInHeader": "", // "である" or "ですます"
             "preferInBody": "ですます",// "である" or "ですます"
             "preferInList": "である",    // "である" or "ですます"
             // 文末以外でも、敬体(ですます調)と常体(である調)を厳しくチェックするかどうか
             "strict": false,
             // preferInでの設定を、"優先" ではなく、"強制" するかどうか
             // 有効化した場合、被検査テキストの表現が統一されていても、
             // preferInオプションで指定された表現に強制する
             "enforcePreferences": false
         }
    }
}

例えば、次の設定を事前に用意していたとします。

{
    "rules": {
        "no-mix-dearu-desumasu": {
             "preferInBody": "ですます",// "である" or "ですます"
             // preferInでの設定を、"優先" ではなく、"強制" するかどうか
             // 有効化した場合、被検査テキストの表現が統一されていても、
             // preferInオプションで指定された表現に強制する
             "enforcePreferences": true
         }
    }
}

従来であれば、"preferInBody": "ですます" が設定されていても、以下の文章はエラーにはなりません。

今日はいい天気である。気持ちの良い朝である。

しかし、事前定義規則により、文章表現を "ですます" に強制したい場合があります。
今回追加したオプションを使用して、"enforcePreferences": true が設定されていると、この文章はエラーになります。

以下の表現に改めれば、エラーは解消されます。

今日はいい天気ですね。気持ちの良い朝です。

@azu
Copy link
Member

azu commented Dec 27, 2023

ありがとうございます。

preferの考慮漏れのような気もするので、新しいオプションなしにprefer*が指定されている場合は強制して良いような気もします。
混在させないという設定は""(空)の指定で実現できてるはずなので、"preferInBody": "ですます" とかを指定した時点で"である"を使ったらエラーにするのは違和感がない気はしています。

そのため、特に今の挙動(混在はしてないけど、preferじゃない書き方が入ってるケースをスルーする)のユースケースが思いつかなければ、今の挙動をenforceの挙動に変えてしまって問題ないと思います。

丁寧にやるならオプションを追加するのが正しそうですが、デフォルトを強制にした場合、ほとんど使われないオプションな気はするのでオプションはなしで良いかなと思いました。(破壊的な変更なので、major updateでリリースする形です)

minor updateで入れたい場合は、今のオプションありで入れて、次のmajorでデフォルトの挙動を変更する感じになりそうですね。

どちらか良いと思いますか?

Comment on lines 85 to 88
return this.dearuCount !== 0 && this.desumasuCount !== 0;
return this.options.isEnforcePreferences
? (this.options.preferDesumasu && this.dearuCount !== 0) ||
(this.options.preferDearu && this.desumasuCount !== 0)
: this.dearuCount !== 0 && this.desumasuCount !== 0;
Copy link
Member

Choose a reason for hiding this comment

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

📝 オプションありのままで行く場合のコメント

if (this.options.isEnforcePreferences) { 
  return ...
}

return ...

三項演算子にまとめる必要があまりなさそう(ロジック的に異なるものなのでブロック的にわかれてた方が、後から片方を消しやすい)なので、ifで分けてearly returnした方が良さそう。

@yo1000
Copy link
Contributor Author

yo1000 commented Dec 28, 2023

@azu
レビューありがとうございます。

丁寧にやるならオプションを追加するのが正しそうですが、デフォルトを強制にした場合、ほとんど使われないオプションな気はするのでオプションはなしで良いかなと思いました。(破壊的な変更なので、major updateでリリースする形です)

minor updateで入れたい場合は、今のオプションありで入れて、次のmajorでデフォルトの挙動を変更する感じになりそうですね。

どちらか良いと思いますか?

Major updateで、破壊的変更として取り込むか、Minor updateで、段階的に取り込むかですが、リリースにかかる期間にどの程度影響を与えるか次第で選択できればと考えています。

いずれを選択した場合も、さほど時間を空けずにリリースされるのであれば、Majorでデフォルト挙動自体を変更してしまう形が好ましく、次期Major updateに他にも何らかの機能追加等が計画されており、少し間を空けるようであればMinorを経由する形が良いです。

リリース時期に関する何らかの計画等はあるでしょうか。選択されたリリース方法に応じた内容で修正を加えようと思います。

@azu
Copy link
Member

azu commented Dec 28, 2023

このルール自体は、マージしたらすぐリリースしてしまうと思います。

presetの方は半年に1度にしていて、次回は1月リリースの予定です。
https://github.com/textlint-ja/textlint-rule-preset-japanese
https://github.com/textlint-ja/textlint-rule-preset-ja-technical-writing

@yo1000
Copy link
Contributor Author

yo1000 commented Dec 28, 2023

@azu
ご回答ありがとうございます。

それでは取り急ぎ、Major updateでリリース可能となるよう修正を加えようと思います。

@yo1000 yo1000 requested a review from azu December 28, 2023 10:09
@yo1000 yo1000 changed the title preferInの設定を優先するのではなく強制するためのオプションを追加 Breaking Change: 文体が統一されていてもpreferInの設定に違反する場合エラーとなるように変更 Dec 28, 2023
@yo1000 yo1000 changed the title Breaking Change: 文体が統一されていてもpreferInの設定に違反する場合エラーとなるように変更 Breaking Change: 文体が統一されていてもpreferIn設定に違反する場合エラーとなるように変更 Dec 28, 2023
package.json Outdated Show resolved Hide resolved
src/MixedChecker.js Show resolved Hide resolved
@azu azu merged commit 0a3926c into textlint-ja:master Dec 28, 2023
2 checks passed
@azu
Copy link
Member

azu commented Dec 28, 2023

ありがとうございます

@azu azu added the Type: Breaking Change Includes breaking changes label Dec 28, 2023
@azu
Copy link
Member

azu commented Dec 28, 2023

https://github.com/textlint-ja/textlint-rule-no-mix-dearu-desumasu/releases/tag/v6.0.0
リリースしました

@yo1000 yo1000 deleted the feature/enforce-preferences branch December 28, 2023 14:27
@azu
Copy link
Member

azu commented Dec 29, 2023

presetのリリースPR(1月にマージの予定)

Canary版:

$ npm install textlint-rule-preset-japanese@next
$ npm install textlint-rule-preset-ja-technical-writing@next

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Breaking Change Includes breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants