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

separatorCharsの設定が無視される #25

Closed
pddg opened this issue Dec 7, 2019 · 4 comments · Fixed by #27
Closed

separatorCharsの設定が無視される #25

pddg opened this issue Dec 7, 2019 · 4 comments · Fixed by #27
Labels
Type: Bug Bug or Bug fixes

Comments

@pddg
Copy link

pddg commented Dec 7, 2019

環境

  • macOS 10.15 Catalina
  • Node 13.3.0
  • textlint 11.5.0
  • textlint-rule-no-doubled-joshi 3.5.3

テストした文字列

1行ごとに異なるファイルに入れています.

右がiPhone,左がAndroidです。
右がiPhone,左がAndroidです.
右がiPhone、左がAndroidです。
右がiPhone、左がAndroidです.
右がiPhone!左がAndroidです!
右がiPhone?左がAndroidです?

.textlintrc

{
    "rules": {
        "no-doubled-joshi": {
            "min_interval": 1,
            "strict": false,
            "allow": [],
            "separatorChars": [""]
        }
    }
}

期待される動作

区切り文字の設定がだけになっているため,全て検出されるはずです.ただし,だけは扱いが特殊なため,

右がiPhone、左がAndroidです。
右がiPhone、左がAndroidです.

の2つを除く全てがエラーになるはずです.

実際の動作

区切り文字に何を指定しても結果に影響しません.例えばのみを指定したとき,

右がiPhone,左がAndroidです。
右がiPhone,左がAndroidです.

だけがエラーとして検出されます.また,区切り文字にを指定しても,結果は変わりませんでした.

私見

TypeScriptにもtextlintにも詳しくないのですが,ソースコードを見る限りseparatorCharsはどこにも使われていないように見えます.
また,全角ピリオドを使う文章では全角カンマ()を読点()の代わりに使用するため,読点のみ扱いが異なる現在の実装では不都合が出る文章も多いかと思いますが,いかがでしょうか?

@azu
Copy link
Member

azu commented Dec 7, 2019

以前のバージョンでは separatorChars がサポートされていましたが、メジャーアップデート時(sentence-splitterを書き直した際)にオプションのサポートをとりあえず外していて、READMEを更新し忘れていたみたいです。

sentence-splitterseparatorChars 相当の部分がハードコードされているので、これをオプションで指定できるようにして、configで設定可能にする必要がありそうです。
https://github.com/azu/sentence-splitter/blob/026643dd81b8fefbaccb2d32e624c77d5e303c9b/src/parser/SeparatorParser.ts#L4

また,全角ピリオドを使う文章では全角カンマ()を読点()の代わりに使用するため,読点のみ扱いが異なる現在の実装では不都合が出る文章も多いかと思いますが,いかがでしょうか?

これは別Issueな気がするので別途Issueを作成してもらえると助かります。

export const is読点Token = (token: KuromojiToken) => {
return token.surface_form === "、" && token.pos === "名詞";
};

https://azu.github.io/morpheme-match/?text=%E5%8F%B3%E3%81%8CiPhone%EF%BC%8C%E5%B7%A6%E3%81%8CAndroid%E3%81%A7%E3%81%99%EF%BC%8E

今は だけしか見てないので、これをconfigで設定可能にするという感じですかね

@azu azu added the Type: Bug Bug or Bug fixes label Dec 7, 2019
@azu
Copy link
Member

azu commented Dec 7, 2019

実装し直した場合に改めて加える形にした方が良さそうなので、今動いてない separatorChars オプションはREADMEから削除しました
0c9d5ec

@pddg
Copy link
Author

pddg commented Dec 7, 2019

ありがとうございます.全角カンマの件については別のissueを作成しました.

@azu
Copy link
Member

azu commented Dec 7, 2019

@pddg https://github.com/textlint-ja/textlint-rule-no-doubled-joshi/releases/tag/3.6.0separatorCharacters オプションを実装しました。
(現在と以前の実装が異なるため、separatorCharsとは別の名前にしています)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug or Bug fixes
Projects
None yet
2 participants